From 99745c22a83dc0eb2fef76732effdf9bb189c0e3f066567fb996e4337f23a4f8 Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Wed, 8 Feb 2023 18:06:01 +0000 Subject: [PATCH] - qemu: Fix umount of /dev in VM private namespace c3f16cea-qemu-cleanup-label-on-umount-failure.patch, 697c16e3-qemu_process-better-debug-message.patch, 5155ab4b-qemu_namespace-nested-mounts-when-umount.patch boo#1207889 OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=966 --- ..._namespace-nested-mounts-when-umount.patch | 54 +++++++++++++++++ ...e3-qemu_process-better-debug-message.patch | 58 +++++++++++++++++++ ...qemu-cleanup-label-on-umount-failure.patch | 28 +++++++++ libvirt.changes | 9 +++ libvirt.spec | 3 + 5 files changed, 152 insertions(+) create mode 100644 5155ab4b-qemu_namespace-nested-mounts-when-umount.patch create mode 100644 697c16e3-qemu_process-better-debug-message.patch create mode 100644 c3f16cea-qemu-cleanup-label-on-umount-failure.patch diff --git a/5155ab4b-qemu_namespace-nested-mounts-when-umount.patch b/5155ab4b-qemu_namespace-nested-mounts-when-umount.patch new file mode 100644 index 0000000..79c5a9e --- /dev/null +++ b/5155ab4b-qemu_namespace-nested-mounts-when-umount.patch @@ -0,0 +1,54 @@ +From 927ddc0ec04e6a838fa807df4546e14f60927949 Mon Sep 17 00:00:00 2001 +From: Michal Privoznik +Date: Tue, 7 Feb 2023 15:06:32 +0100 +Subject: [PATCH 3/3] qemu_namespace: Deal with nested mounts when umount()-ing + /dev + +In one of recent commits (v9.0.0-rc1~106) I've made our QEMU +namespace code umount the original /dev. One of the reasons was +enhanced security, because previously we just mounted a tmpfs +over the original /dev. Thus a malicious QEMU could just +umount("/dev") and it would get to the original /dev with all +nodes. + +Now, on some systems this introduced a regression: + + failed to umount devfs on /dev: Device or resource busy + +But how this could be? We've moved all file systems mounted under +/dev to a temporary location. Or have we? As it turns out, not +quite. If there are two file systems mounted on the same target, +e.g. like this: + + mount -t tmpfs tmpfs /dev/shm/ && mount -t tmpfs tmpfs /dev/shm/ + +then only the top most (i.e. the last one) is moved. See +qemuDomainUnshareNamespace() for more info. + +Now, we could enhance our code to deal with these "doubled" mount +points. Or, since it is the top most file system that is +accessible anyways (and this one is preserved), we can +umount("/dev") in a recursive fashion. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2167302 +Fixes: 379c0ce4bfed8733dfbde557c359eecc5474ce38 +Signed-off-by: Michal Privoznik +Reviewed-by: Jim Fehlig +(cherry picked from commit 5155ab4b2a704285505dfea6ffee8b980fdaa29e) +--- + src/qemu/qemu_namespace.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +Index: libvirt-9.0.0/src/qemu/qemu_namespace.c +=================================================================== +--- libvirt-9.0.0.orig/src/qemu/qemu_namespace.c ++++ libvirt-9.0.0/src/qemu/qemu_namespace.c +@@ -777,7 +777,7 @@ qemuDomainUnshareNamespace(virQEMUDriver + } + + #if defined(__linux__) +- if (umount("/dev") < 0) { ++ if (umount2("/dev", MNT_DETACH) < 0) { + virReportSystemError(errno, "%s", _("failed to umount devfs on /dev")); + goto cleanup; + } diff --git a/697c16e3-qemu_process-better-debug-message.patch b/697c16e3-qemu_process-better-debug-message.patch new file mode 100644 index 0000000..d5caf3d --- /dev/null +++ b/697c16e3-qemu_process-better-debug-message.patch @@ -0,0 +1,58 @@ +From dabff45cbba94d6dedf7319ba3f225d4bebef010 Mon Sep 17 00:00:00 2001 +From: Michal Privoznik +Date: Tue, 7 Feb 2023 10:34:40 +0100 +Subject: [PATCH 2/3] qemu_process: Produce better debug message wrt domain + namespaces + +When going through debug log of a domain startup process, one can +meet the following line: + + debug : qemuProcessLaunch:7668 : Building mount namespace + +But this is in fact wrong. Firstly, domain namespaces are just +enabled in domain's privateData. Secondly, the debug message says +nothing about actual state of namespace - whether it was enabled +or not. + +Therefore, move the debug printing into +qemuProcessEnableDomainNamespaces() and tweak it so that the +actual value is reflected. + +Signed-off-by: Michal Privoznik +Reviewed-by: Jim Fehlig +(cherry picked from commit 697c16e39ae9a9e18ce7cad0729bf2293b12a307) +--- + src/qemu/qemu_process.c | 8 ++++++-- + 1 file changed, 6 insertions(+), 2 deletions(-) + +Index: libvirt-9.0.0/src/qemu/qemu_process.c +=================================================================== +--- libvirt-9.0.0.orig/src/qemu/qemu_process.c ++++ libvirt-9.0.0/src/qemu/qemu_process.c +@@ -7399,11 +7399,17 @@ qemuProcessEnableDomainNamespaces(virQEM + virDomainObj *vm) + { + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); ++ const char *state = "disabled"; + + if (virBitmapIsBitSet(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) && + qemuDomainEnableNamespace(vm, QEMU_DOMAIN_NS_MOUNT) < 0) + return -1; + ++ if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) ++ state = "enabled"; ++ ++ VIR_DEBUG("Mount namespace for domain name=%s is %s", ++ vm->def->name, state); + return 0; + } + +@@ -7720,8 +7726,6 @@ qemuProcessLaunch(virConnectPtr conn, + + qemuDomainLogContextMarkPosition(logCtxt); + +- VIR_DEBUG("Building mount namespace"); +- + if (qemuProcessEnableDomainNamespaces(driver, vm) < 0) + goto cleanup; + diff --git a/c3f16cea-qemu-cleanup-label-on-umount-failure.patch b/c3f16cea-qemu-cleanup-label-on-umount-failure.patch new file mode 100644 index 0000000..d68b5b7 --- /dev/null +++ b/c3f16cea-qemu-cleanup-label-on-umount-failure.patch @@ -0,0 +1,28 @@ +From aa144c00c1b8f1deee6f80f8de076d5bfac72811 Mon Sep 17 00:00:00 2001 +From: Jim Fehlig +Date: Mon, 6 Feb 2023 10:40:12 -0700 +Subject: [PATCH 1/3] qemu: Jump to cleanup label on umount failure + +Similar to other error paths in qemuDomainUnshareNamespace(), jump to +the cleanup label on umount error instead of directly returning -1. + +Signed-off-by: Jim Fehlig +Reviewed-by: Michal Privoznik +(cherry picked from commit c3f16cea3bef578c498c720aa90c677ee9511e2f) +--- + src/qemu/qemu_namespace.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +Index: libvirt-9.0.0/src/qemu/qemu_namespace.c +=================================================================== +--- libvirt-9.0.0.orig/src/qemu/qemu_namespace.c ++++ libvirt-9.0.0/src/qemu/qemu_namespace.c +@@ -779,7 +779,7 @@ qemuDomainUnshareNamespace(virQEMUDriver + #if defined(__linux__) + if (umount("/dev") < 0) { + virReportSystemError(errno, "%s", _("failed to umount devfs on /dev")); +- return -1; ++ goto cleanup; + } + #endif /* !defined(__linux__) */ + diff --git a/libvirt.changes b/libvirt.changes index 3bdfc97..7d147ff 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Wed Feb 8 18:01:55 UTC 2023 - James Fehlig + +- qemu: Fix umount of /dev in VM private namespace + c3f16cea-qemu-cleanup-label-on-umount-failure.patch, + 697c16e3-qemu_process-better-debug-message.patch, + 5155ab4b-qemu_namespace-nested-mounts-when-umount.patch + boo#1207889 + ------------------------------------------------------------------- Tue Jan 17 17:33:00 UTC 2023 - James Fehlig diff --git a/libvirt.spec b/libvirt.spec index 48e1012..b14f90b 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -306,6 +306,9 @@ Source100: %{name}-rpmlintrc # Upstream patches Patch0: ef482951-apparmor-Allow-umount-dev.patch Patch1: d6a8b9ee-qemu-Fix-managed-no-when-creating-ethdev.patch +Patch2: c3f16cea-qemu-cleanup-label-on-umount-failure.patch +Patch3: 697c16e3-qemu_process-better-debug-message.patch +Patch4: 5155ab4b-qemu_namespace-nested-mounts-when-umount.patch # Patches pending upstream review Patch100: libxl-dom-reset.patch Patch101: network-don-t-use-dhcp-authoritative-on-static-netwo.patch