- 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

5155ab4b-qemu_namespace-nested-mounts-when-umount.patch

@@ -0,0 +1,54 @@
+From 927ddc0ec04e6a838fa807df4546e14f60927949 Mon Sep 17 00:00:00 2001
+From: Michal Privoznik <mprivozn@redhat.com>
+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 <mprivozn@redhat.com>
+Reviewed-by: Jim Fehlig <jfehlig@suse.com>
+(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;
+     }

697c16e3-qemu_process-better-debug-message.patch

@@ -0,0 +1,58 @@
+From dabff45cbba94d6dedf7319ba3f225d4bebef010 Mon Sep 17 00:00:00 2001
+From: Michal Privoznik <mprivozn@redhat.com>
+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 <mprivozn@redhat.com>
+Reviewed-by: Jim Fehlig <jfehlig@suse.com>
+(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;
+ 

c3f16cea-qemu-cleanup-label-on-umount-failure.patch

@@ -0,0 +1,28 @@
+From aa144c00c1b8f1deee6f80f8de076d5bfac72811 Mon Sep 17 00:00:00 2001
+From: Jim Fehlig <jfehlig@suse.com>
+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 <jfehlig@suse.com>
+Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
+(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__) */
+ 

libvirt.changes

@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Wed Feb  8 18:01:55 UTC 2023 - James Fehlig <jfehlig@suse.com>
+
+- 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 <jfehlig@suse.com>
 

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