* [pve-devel] [PATCH v2 container 0/2] fix #5160: fix move_mount regression for mount point hotplug @ 2024-03-25 17:28 Filip Schauer 2024-03-25 17:28 ` [pve-devel] [PATCH v2 container 1/2] " Filip Schauer 2024-03-25 17:28 ` [pve-devel] [PATCH v2 container 2/2] fix undef warning when apparmor changeprofile fails Filip Schauer 0 siblings, 2 replies; 5+ messages in thread From: Filip Schauer @ 2024-03-25 17:28 UTC (permalink / raw) To: pve-devel Changes since v1: * Fix loading of apparmor profile not working in postinst, since the profile is not found by dh_apparmor. This is fixed by moving pve-container-mounthotplug out of the pve subdirectory. * Fix a perl undef warning when apparmor changeprofile fails. Filip Schauer (2): fix #5160: fix move_mount regression for mount point hotplug fix undef warning when apparmor changeprofile fails debian/rules | 3 +++ src/Makefile | 3 +++ src/PVE/LXC.pm | 5 +++-- src/pve-container-debug@.service | 1 + src/pve-container-mounthotplug | 7 +++++++ src/pve-container@.service | 1 + 6 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 src/pve-container-mounthotplug -- 2.39.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH v2 container 1/2] fix #5160: fix move_mount regression for mount point hotplug 2024-03-25 17:28 [pve-devel] [PATCH v2 container 0/2] fix #5160: fix move_mount regression for mount point hotplug Filip Schauer @ 2024-03-25 17:28 ` Filip Schauer 2024-04-09 8:23 ` Wolfgang Bumiller 2024-03-25 17:28 ` [pve-devel] [PATCH v2 container 2/2] fix undef warning when apparmor changeprofile fails Filip Schauer 1 sibling, 1 reply; 5+ messages in thread From: Filip Schauer @ 2024-03-25 17:28 UTC (permalink / raw) To: pve-devel Set up an Apparmor profile to allow moving mounts for mount point hotplug. This fixes a regression caused by kernel commit 157a3537d6 ("apparmor: Fix regression in mount mediation") The commit introduced move_mount mediation, which now requires move_mount to be allowed in the Apparmor profile. Although it is allowed for most paths in the /usr/bin/lxc-start profile, move_mount is called with a file descriptor instead of a path in mountpoint_insert_staged, thus it is not affected by the allow rules in /etc/apparmor.d/abstractions/lxc/container-base. To fix this, introduce a new Apparmor profile to allow move_mount on every mount, specifically for mount point hotplug. Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- debian/rules | 3 +++ src/Makefile | 3 +++ src/PVE/LXC.pm | 2 +- src/pve-container-debug@.service | 1 + src/pve-container-mounthotplug | 7 +++++++ src/pve-container@.service | 1 + 6 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 src/pve-container-mounthotplug diff --git a/debian/rules b/debian/rules index d999152..f7edccf 100755 --- a/debian/rules +++ b/debian/rules @@ -14,3 +14,6 @@ override_dh_installsystemd: dh_installsystemd -ppve-container --no-start --no-enable --no-restart-after-upgrade -r 'system-pve\x2dcontainer.slice' + +override_dh_install: + dh_apparmor -p pve-container --profile-name=pve-container-mounthotplug diff --git a/src/Makefile b/src/Makefile index 5a7a82e..e0b7734 100644 --- a/src/Makefile +++ b/src/Makefile @@ -4,6 +4,7 @@ PREFIX=${DESTDIR}/usr BINDIR=${PREFIX}/bin LIBDIR=${PREFIX}/lib SBINDIR=${PREFIX}/sbin +ETCDIR=${DESTDIR}/etc MANDIR=${PREFIX}/share/man DOCDIR=${PREFIX}/share/doc/${PACKAGE} LXC_SCRIPT_DIR=${PREFIX}/share/lxc @@ -13,6 +14,7 @@ LXC_CONFIG_DIR=${LXC_SCRIPT_DIR}/config LXC_COMMON_CONFIG_DIR=${LXC_CONFIG_DIR}/common.conf.d LXC_USERNS_CONFIG_DIR=${LXC_CONFIG_DIR}/userns.conf.d SERVICEDIR=${DESTDIR}/lib/systemd/system +APPARMORDDIR=${ETCDIR}/apparmor.d PODDIR=${DOCDIR}/pod MAN1DIR=${MANDIR}/man1/ MAN5DIR=${MANDIR}/man5/ @@ -73,6 +75,7 @@ install: pct lxc-pve.conf pct.1 pct.conf.5 pct.bash-completion pct.zsh-completio gzip -9 ${MAN5DIR}/pct.conf.5 cd ${MAN5DIR}; ln -s pct.conf.5.gz ct.conf.5.gz install -D -m 0644 10-pve-ct-inotify-limits.conf ${LIBDIR}/sysctl.d/10-pve-ct-inotify-limits.conf + install -D -m 0644 pve-container-mounthotplug ${APPARMORDDIR}/pve-container-mounthotplug pve-userns.seccomp: /usr/share/lxc/config/common.seccomp cp $< $@ diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm index 7883cfb..7db4833 100644 --- a/src/PVE/LXC.pm +++ b/src/PVE/LXC.pm @@ -1974,7 +1974,7 @@ sub mountpoint_hotplug :prototype($$$$$) { my $dir = get_staging_mount_path($opt); # Now switch our apparmor profile before mounting: - my $data = 'changeprofile /usr/bin/lxc-start'; + my $data = 'changeprofile pve-container-mounthotplug'; if (syswrite($aa_fd, $data, length($data)) != length($data)) { die "failed to change apparmor profile: $!\n"; } diff --git a/src/pve-container-debug@.service b/src/pve-container-debug@.service index 7cfebaa..cd0895c 100644 --- a/src/pve-container-debug@.service +++ b/src/pve-container-debug@.service @@ -13,6 +13,7 @@ Type=simple Delegate=yes KillMode=mixed TimeoutStopSec=120s +ExecStartPre=/lib/apparmor/profile-load pve-container-mounthotplug ExecStart=/usr/bin/lxc-start -F -n %i -o /dev/stderr -l DEBUG ExecStop=/usr/share/lxc/pve-container-stop-wrapper %i # Environment=BOOTUP=serial diff --git a/src/pve-container-mounthotplug b/src/pve-container-mounthotplug new file mode 100644 index 0000000..e6f3903 --- /dev/null +++ b/src/pve-container-mounthotplug @@ -0,0 +1,7 @@ +#include <tunables/global> + +profile pve-container-mounthotplug flags=(attach_disconnected) { + #include <abstractions/lxc/start-container> + + mount options=(move), +} diff --git a/src/pve-container@.service b/src/pve-container@.service index fdc373e..1437858 100644 --- a/src/pve-container@.service +++ b/src/pve-container@.service @@ -13,6 +13,7 @@ Type=simple Delegate=yes KillMode=mixed TimeoutStopSec=120s +ExecStartPre=/lib/apparmor/profile-load pve-container-mounthotplug ExecStart=/usr/bin/lxc-start -F -n %i ExecStop=/usr/share/lxc/pve-container-stop-wrapper %i # Environment=BOOTUP=serial -- 2.39.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH v2 container 1/2] fix #5160: fix move_mount regression for mount point hotplug 2024-03-25 17:28 ` [pve-devel] [PATCH v2 container 1/2] " Filip Schauer @ 2024-04-09 8:23 ` Wolfgang Bumiller 2024-04-09 9:27 ` Filip Schauer 0 siblings, 1 reply; 5+ messages in thread From: Wolfgang Bumiller @ 2024-04-09 8:23 UTC (permalink / raw) To: Filip Schauer; +Cc: pve-devel looks mostly good, just the ExecStartPre= lines in the service files should be dropped On Mon, Mar 25, 2024 at 06:28:28PM +0100, Filip Schauer wrote: > Set up an Apparmor profile to allow moving mounts for mount point > hotplug. > > This fixes a regression caused by > kernel commit 157a3537d6 ("apparmor: Fix regression in mount mediation") > > The commit introduced move_mount mediation, which now requires > move_mount to be allowed in the Apparmor profile. Although it is allowed > for most paths in the /usr/bin/lxc-start profile, move_mount is called > with a file descriptor instead of a path in mountpoint_insert_staged, > thus it is not affected by the allow rules in > /etc/apparmor.d/abstractions/lxc/container-base. > > To fix this, introduce a new Apparmor profile to allow move_mount on > every mount, specifically for mount point hotplug. > > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > debian/rules | 3 +++ > src/Makefile | 3 +++ > src/PVE/LXC.pm | 2 +- > src/pve-container-debug@.service | 1 + > src/pve-container-mounthotplug | 7 +++++++ > src/pve-container@.service | 1 + > 6 files changed, 16 insertions(+), 1 deletion(-) > create mode 100644 src/pve-container-mounthotplug > > diff --git a/debian/rules b/debian/rules > index d999152..f7edccf 100755 > --- a/debian/rules > +++ b/debian/rules > @@ -14,3 +14,6 @@ > > override_dh_installsystemd: > dh_installsystemd -ppve-container --no-start --no-enable --no-restart-after-upgrade -r 'system-pve\x2dcontainer.slice' > + > +override_dh_install: > + dh_apparmor -p pve-container --profile-name=pve-container-mounthotplug > diff --git a/src/Makefile b/src/Makefile > index 5a7a82e..e0b7734 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -4,6 +4,7 @@ PREFIX=${DESTDIR}/usr > BINDIR=${PREFIX}/bin > LIBDIR=${PREFIX}/lib > SBINDIR=${PREFIX}/sbin > +ETCDIR=${DESTDIR}/etc > MANDIR=${PREFIX}/share/man > DOCDIR=${PREFIX}/share/doc/${PACKAGE} > LXC_SCRIPT_DIR=${PREFIX}/share/lxc > @@ -13,6 +14,7 @@ LXC_CONFIG_DIR=${LXC_SCRIPT_DIR}/config > LXC_COMMON_CONFIG_DIR=${LXC_CONFIG_DIR}/common.conf.d > LXC_USERNS_CONFIG_DIR=${LXC_CONFIG_DIR}/userns.conf.d > SERVICEDIR=${DESTDIR}/lib/systemd/system > +APPARMORDDIR=${ETCDIR}/apparmor.d > PODDIR=${DOCDIR}/pod > MAN1DIR=${MANDIR}/man1/ > MAN5DIR=${MANDIR}/man5/ > @@ -73,6 +75,7 @@ install: pct lxc-pve.conf pct.1 pct.conf.5 pct.bash-completion pct.zsh-completio > gzip -9 ${MAN5DIR}/pct.conf.5 > cd ${MAN5DIR}; ln -s pct.conf.5.gz ct.conf.5.gz > install -D -m 0644 10-pve-ct-inotify-limits.conf ${LIBDIR}/sysctl.d/10-pve-ct-inotify-limits.conf > + install -D -m 0644 pve-container-mounthotplug ${APPARMORDDIR}/pve-container-mounthotplug > > pve-userns.seccomp: /usr/share/lxc/config/common.seccomp > cp $< $@ > diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm > index 7883cfb..7db4833 100644 > --- a/src/PVE/LXC.pm > +++ b/src/PVE/LXC.pm > @@ -1974,7 +1974,7 @@ sub mountpoint_hotplug :prototype($$$$$) { > my $dir = get_staging_mount_path($opt); > > # Now switch our apparmor profile before mounting: > - my $data = 'changeprofile /usr/bin/lxc-start'; > + my $data = 'changeprofile pve-container-mounthotplug'; > if (syswrite($aa_fd, $data, length($data)) != length($data)) { > die "failed to change apparmor profile: $!\n"; > } > diff --git a/src/pve-container-debug@.service b/src/pve-container-debug@.service > index 7cfebaa..cd0895c 100644 > --- a/src/pve-container-debug@.service > +++ b/src/pve-container-debug@.service > @@ -13,6 +13,7 @@ Type=simple > Delegate=yes > KillMode=mixed > TimeoutStopSec=120s > +ExecStartPre=/lib/apparmor/profile-load pve-container-mounthotplug > ExecStart=/usr/bin/lxc-start -F -n %i -o /dev/stderr -l DEBUG > ExecStop=/usr/share/lxc/pve-container-stop-wrapper %i > # Environment=BOOTUP=serial ^ This hunk should be dropped. The entry in d/rules is enough, and this is the wrong place anyway, as this is triggered when starting a new container, and not when hotplugging. > diff --git a/src/pve-container-mounthotplug b/src/pve-container-mounthotplug > new file mode 100644 > index 0000000..e6f3903 > --- /dev/null > +++ b/src/pve-container-mounthotplug > @@ -0,0 +1,7 @@ > +#include <tunables/global> > + > +profile pve-container-mounthotplug flags=(attach_disconnected) { > + #include <abstractions/lxc/start-container> > + > + mount options=(move), > +} > diff --git a/src/pve-container@.service b/src/pve-container@.service > index fdc373e..1437858 100644 > --- a/src/pve-container@.service > +++ b/src/pve-container@.service > @@ -13,6 +13,7 @@ Type=simple > Delegate=yes > KillMode=mixed > TimeoutStopSec=120s > +ExecStartPre=/lib/apparmor/profile-load pve-container-mounthotplug > ExecStart=/usr/bin/lxc-start -F -n %i > ExecStop=/usr/share/lxc/pve-container-stop-wrapper %i > # Environment=BOOTUP=serial ^ same for this one ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH v2 container 1/2] fix #5160: fix move_mount regression for mount point hotplug 2024-04-09 8:23 ` Wolfgang Bumiller @ 2024-04-09 9:27 ` Filip Schauer 0 siblings, 0 replies; 5+ messages in thread From: Filip Schauer @ 2024-04-09 9:27 UTC (permalink / raw) To: Wolfgang Bumiller; +Cc: pve-devel Agreed. This appear to be a leftover from testing. A patch v3 with this removed is available: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062693.html On 09/04/2024 10:23, Wolfgang Bumiller wrote: >> diff --gita/src/pve-container-debug@.service b/src/pve-container-debug@.service >> index 7cfebaa..cd0895c 100644 >> ---a/src/pve-container-debug@.service >> +++b/src/pve-container-debug@.service >> @@ -13,6 +13,7 @@ Type=simple >> Delegate=yes >> KillMode=mixed >> TimeoutStopSec=120s >> +ExecStartPre=/lib/apparmor/profile-load pve-container-mounthotplug >> ExecStart=/usr/bin/lxc-start -F -n %i -o /dev/stderr -l DEBUG >> ExecStop=/usr/share/lxc/pve-container-stop-wrapper %i >> # Environment=BOOTUP=serial > ^ This hunk should be dropped. The entry in d/rules is enough, and this > is the wrong place anyway, as this is triggered when starting a new > container, and not when hotplugging. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH v2 container 2/2] fix undef warning when apparmor changeprofile fails 2024-03-25 17:28 [pve-devel] [PATCH v2 container 0/2] fix #5160: fix move_mount regression for mount point hotplug Filip Schauer 2024-03-25 17:28 ` [pve-devel] [PATCH v2 container 1/2] " Filip Schauer @ 2024-03-25 17:28 ` Filip Schauer 1 sibling, 0 replies; 5+ messages in thread From: Filip Schauer @ 2024-03-25 17:28 UTC (permalink / raw) To: pve-devel Fix a "Use of uninitialized value in numeric ne (!=)" warning when syswrite returns undef when trying to change the apparmor profile. Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- src/PVE/LXC.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm index 7db4833..88a9d6f 100644 --- a/src/PVE/LXC.pm +++ b/src/PVE/LXC.pm @@ -1975,7 +1975,8 @@ sub mountpoint_hotplug :prototype($$$$$) { # Now switch our apparmor profile before mounting: my $data = 'changeprofile pve-container-mounthotplug'; - if (syswrite($aa_fd, $data, length($data)) != length($data)) { + my $data_written = syswrite($aa_fd, $data, length($data)); + if (!defined($data_written) || $data_written != length($data)) { die "failed to change apparmor profile: $!\n"; } # Check errors on close as well: -- 2.39.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-09 9:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-25 17:28 [pve-devel] [PATCH v2 container 0/2] fix #5160: fix move_mount regression for mount point hotplug Filip Schauer 2024-03-25 17:28 ` [pve-devel] [PATCH v2 container 1/2] " Filip Schauer 2024-04-09 8:23 ` Wolfgang Bumiller 2024-04-09 9:27 ` Filip Schauer 2024-03-25 17:28 ` [pve-devel] [PATCH v2 container 2/2] fix undef warning when apparmor changeprofile fails Filip Schauer
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.Service provided by Proxmox Server Solutions GmbH | Privacy | Legal