[PATCH 1/2] qemu_namespace: Don't unlink paths from cgroupDeviceACL

[Ján Tomko]

On a Monday in 2022, Michal Privoznik wrote:
>When building namespace for a domain there are couple of devices
>that are created independent of domain config (see
>qemuDomainPopulateDevices()). The idea behind is that these
>devices are crucial for QEMU or one of its libraries, or user is
>passing through a device and wants us to create it in the
>namespace too.  That's the reason that these devices are allowed
>in the devices CGroup controller as well.
>
>However, during unplug it may happen that a device is configured
>to use one of such devices and since we remove /dev nodes on
>hotplug we would remove such device too. For example,
>/dev/urandom belongs onto the list of implicit devices and users
>can hotplug and hotunplug an RNG device with /dev/urandom as
>backend.
>
>The fix is fortunately simple - just consult the list of implicit
>devices before removing the device from the namespace.
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/qemu/qemu_namespace.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
>index 3b41d72630..1132fd04e5 100644
>--- a/src/qemu/qemu_namespace.c
>+++ b/src/qemu/qemu_namespace.c
>@@ -1364,6 +1364,8 @@ qemuNamespaceUnlinkPaths(virDomainObj *vm,
>         if (STRPREFIX(path, QEMU_DEVPREFIX)) {
>             GStrv mount;
>             bool inSubmount = false;
>+            const char *const *devices = (const char *const *)cfg->cgroupDeviceACL;
>+            bool inDevices = false;
>
>             for (mount = devMountsPath; *mount; mount++) {
>                 if (STREQ(*mount, "/dev"))
>@@ -1375,8 +1377,23 @@ qemuNamespaceUnlinkPaths(virDomainObj *vm,
>                 }
>             }
>
>-            if (!inSubmount)
>-                unlinkPaths = g_slist_prepend(unlinkPaths, g_strdup(path));
>+            if (inSubmount)
>+                continue;
>+
>+            if (!devices)
>+                devices = defaultDeviceACL;
>+

>+            for (; devices; devices++) {
>+                if (STREQ(path, *devices)) {
>+                    inDevices = true;
>+                    break;
>+                }
>+            }
>+
>+            if (inDevices)
>+                continue;
>+

something like:

if (g_strv_contains(devices, path))
     continue;

should do the same without the need for the bool variable.

(Not sure how to nicely eliminate the other one)

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano

>+            unlinkPaths = g_slist_prepend(unlinkPaths, g_strdup(path));
>         }
>     }
>
>-- 
>2.34.1
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220314/61653e9c/attachment.sig>