Blob Blame History Raw
From fd06fc3affcda0d7af1721c26915b8d87e0b2614 Mon Sep 17 00:00:00 2001
Message-Id: <fd06fc3affcda0d7af1721c26915b8d87e0b2614@dist-git>
From: Michal Privoznik <mprivozn@redhat.com>
Date: Tue, 7 Feb 2023 15:06:32 +0100
Subject: [PATCH] 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)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 5769a4dfe0..5fc043bd62 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -777,7 +777,7 @@ qemuDomainUnshareNamespace(virQEMUDriverConfig *cfg,
     }
 
 #if defined(__linux__)
-    if (umount("/dev") < 0) {
+    if (umount2("/dev", MNT_DETACH) < 0) {
         virReportSystemError(errno, "%s", _("failed to umount devfs on /dev"));
         return -1;
     }
-- 
2.39.1