A domain that runs with TCG emulation does not need kvm device, so drop
it from default device ACL.

To dynamically add devices to defaultDeviceACL, make it a GSList. This
variable will be initialized when qemu driver is initalized.

Lastly, dynamically append /dev/kvm to default ACL only if the domain is
of type VIR_DOMAIN_VIRT_KVM.

Signed-off-by: Praveen K Paladugu <[email protected]>
---
 src/qemu/qemu.conf.in              |  3 +-
 src/qemu/qemu_cgroup.c             | 52 ++++++++++++++++++++++--------
 src/qemu/qemu_cgroup.h             |  5 ++-
 src/qemu/qemu_conf.c               | 14 ++++++--
 src/qemu/qemu_conf.h               |  2 +-
 src/qemu/qemu_driver.c             |  4 +++
 src/qemu/qemu_namespace.c          | 12 +++----
 src/qemu/qemu_process.c            |  6 ++--
 src/qemu/test_libvirtd_qemu.aug.in |  3 +-
 9 files changed, 71 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
index fc91ba8f08..0a8abd9544 100644
--- a/src/qemu/qemu.conf.in
+++ b/src/qemu/qemu.conf.in
@@ -618,8 +618,7 @@
 #cgroup_device_acl = [
 #    "/dev/null", "/dev/full", "/dev/zero",
 #    "/dev/random", "/dev/urandom",
-#    "/dev/ptmx", "/dev/kvm",
-#    "/dev/userfaultfd"
+#    "/dev/ptmx", "/dev/userfaultfd"
 #]
 #
 # RDMA migration requires the following extra files to be added to the list:
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index f10976c2b0..b2dcefd81e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -38,17 +38,38 @@
 
 VIR_LOG_INIT("qemu.qemu_cgroup");
 
-const char *const defaultDeviceACL[] = {
+GSList *defaultDeviceACL;
+
+const char *const _defaultDeviceACL[] = {
     "/dev/null", "/dev/full", "/dev/zero",
     "/dev/random", "/dev/urandom",
-    "/dev/ptmx", "/dev/kvm",
-    "/dev/userfaultfd",
+    "/dev/ptmx", "/dev/userfaultfd",
     NULL,
 };
 #define DEVICE_PTY_MAJOR 136
 #define DEVICE_SND_MAJOR 116
 
 
+void
+initDefaultDeviceACL(void)
+{
+    size_t i;
+
+    for (i = 0; _defaultDeviceACL[i] != NULL; i++) {
+        defaultDeviceACL = g_slist_append(defaultDeviceACL,
+                                            g_strdup(_defaultDeviceACL[i]));
+    }
+}
+
+void
+updateDefaultDeviceACL(virDomainObj *vm)
+{
+    if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) {
+        defaultDeviceACL = g_slist_append(defaultDeviceACL,
+                                                g_strdup("/dev/kvm"));
+    }
+}
+
 static int
 qemuCgroupAllowDevicePath(virDomainObj *vm,
                           const char *path,
@@ -71,19 +92,19 @@ qemuCgroupAllowDevicePath(virDomainObj *vm,
 
 static int
 qemuCgroupAllowDevicesPaths(virDomainObj *vm,
-                            const char *const *deviceACL,
+                            GSList *deviceACL,
                             int perms,
                             bool ignoreEacces)
 {
-    size_t i;
+    GSList *cur = NULL;
 
-    for (i = 0; deviceACL[i] != NULL; i++) {
-        if (!virFileExists(deviceACL[i])) {
-            VIR_DEBUG("Ignoring non-existent device %s", deviceACL[i]);
+    for (cur = deviceACL; cur; cur = g_slist_next(cur)) {
+        if (!virFileExists(cur->data)) {
+            VIR_DEBUG("Ignoring non-existent device %s", (char *)cur->data);
             continue;
         }
 
-        if (qemuCgroupAllowDevicePath(vm, deviceACL[i], perms, ignoreEacces) < 
0)
+        if (qemuCgroupAllowDevicePath(vm, cur->data, perms, ignoreEacces) < 0)
             return -1;
     }
 
@@ -99,13 +120,13 @@ qemuCgroupDenyDevicePath(virDomainObj *vm,
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
-    const char *const *deviceACL = (const char *const *)cfg->cgroupDeviceACL;
     int ret;
+    GSList *deviceACL = cfg->cgroupDeviceACL;
 
     if (!deviceACL)
         deviceACL = defaultDeviceACL;
 
-    if (g_strv_contains(deviceACL, path)) {
+    if (g_slist_find(deviceACL, path)) {
         VIR_DEBUG("Skipping deny of path %s in CGroups because it's in 
cgroupDeviceACL",
                   path);
         return 0;
@@ -556,8 +577,11 @@ qemuSetupMemoryDevicesCgroup(virDomainObj *vm,
                              virDomainMemoryDef *mem)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
-    const char *const sgxPaths[] = { QEMU_DEV_SGX_VEPVC,
-                                     QEMU_DEV_SGX_PROVISION, NULL };
+    g_autoptr(virGSListString) sgxPaths = NULL;
+
+    sgxPaths = g_slist_append(sgxPaths, g_strdup(QEMU_DEV_SGX_VEPVC));
+    sgxPaths = g_slist_append(sgxPaths, g_strdup(QEMU_DEV_SGX_PROVISION));
+    sgxPaths = g_slist_append(sgxPaths, NULL);
 
     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
         return 0;
@@ -758,7 +782,7 @@ qemuSetupDevicesCgroup(virDomainObj *vm)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
-    const char *const *deviceACL = (const char *const *) cfg->cgroupDeviceACL;
+    GSList *deviceACL = cfg->cgroupDeviceACL;
     int rv = -1;
     size_t i;
 
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 3668034cde..402120a8f2 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -66,4 +66,7 @@ struct _qemuCgroupEmulatorAllNodesData {
     char *emulatorMemMask;
 };
 
-extern const char *const defaultDeviceACL[];
+void updateDefaultDeviceACL(virDomainObj *vm);
+void initDefaultDeviceACL(void);
+
+extern GSList *defaultDeviceACL;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 242955200a..a19a86cd70 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -345,7 +345,8 @@ static void virQEMUDriverConfigDispose(void *obj)
 
     virBitmapFree(cfg->namespaces);
 
-    g_strfreev(cfg->cgroupDeviceACL);
+    g_slist_free(cfg->cgroupDeviceACL);
+    cfg->cgroupDeviceACL = NULL;
     g_free(cfg->uri);
 
     g_free(cfg->configBaseDir);
@@ -1068,6 +1069,7 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfig 
*cfg,
     g_auto(GStrv) namespaces = NULL;
     g_autofree char *user = NULL;
     g_autofree char *group = NULL;
+    char **cgroupDeviceACL = NULL;
     size_t i, j;
 
     if (virConfGetValueStringList(conf, "security_driver", true, 
&cfg->securityDriverNames) < 0)
@@ -1125,9 +1127,17 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfig 
*cfg,
     }
 
     if (virConfGetValueStringList(conf, "cgroup_device_acl", false,
-                                  &cfg->cgroupDeviceACL) < 0)
+                                  &cgroupDeviceACL) < 0)
         return -1;
 
+    if (cgroupDeviceACL) {
+        for (i = 0; cgroupDeviceACL[i] != NULL; i++) {
+            cfg->cgroupDeviceACL = g_slist_append(cfg->cgroupDeviceACL,
+                                                  
g_strdup(cgroupDeviceACL[i]));
+        }
+        g_strfreev(cgroupDeviceACL);
+    }
+
     if (virConfGetValueInt(conf, "seccomp_sandbox", &cfg->seccompSandbox) < 0)
         return -1;
 
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index edb65c99f4..bef198c2c8 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -96,7 +96,7 @@ struct _virQEMUDriverConfig {
     bool rememberOwner;
 
     int cgroupControllers;
-    char **cgroupDeviceACL;
+    GSList *cgroupDeviceACL;
 
     /* These five directories are ones libvirtd uses (so must be root:root
      * to avoid security risk from QEMU processes */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ac72ea5cb0..a5fff3dfb1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -52,6 +52,7 @@
 #include "qemu_saveimage.h"
 #include "qemu_snapshot.h"
 #include "qemu_validate.h"
+#include "qemu_cgroup.h"
 
 #include "virerror.h"
 #include "virlog.h"
@@ -910,6 +911,8 @@ qemuStateInitialize(bool privileged,
     };
     virDomainDriverAutoStart(qemu_driver->domains, &autostartCfg);
 
+    initDefaultDeviceACL();
+
     return VIR_DRV_STATE_INIT_COMPLETE;
 
  error:
@@ -1037,6 +1040,7 @@ qemuStateCleanup(void)
     if (qemu_driver->lockFD != -1)
         virPidFileRelease(qemu_driver->config->stateDir, "driver", 
qemu_driver->lockFD);
 
+    g_slist_free(defaultDeviceACL);
     virObjectUnref(qemu_driver->config);
     virMutexDestroy(&qemu_driver->lock);
     VIR_FREE(qemu_driver);
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index f72da83929..74e2730d2d 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -212,14 +212,14 @@ static int
 qemuDomainPopulateDevices(virQEMUDriverConfig *cfg,
                           GSList **paths)
 {
-    const char *const *devices = (const char *const *) cfg->cgroupDeviceACL;
-    size_t i;
+    GSList *devices = cfg->cgroupDeviceACL;
+    GSList *cur = NULL;
 
     if (!devices)
         devices = defaultDeviceACL;
 
-    for (i = 0; devices[i]; i++) {
-        *paths = g_slist_prepend(*paths, g_strdup(devices[i]));
+    for (cur = devices; cur; cur = g_slist_next(cur)) {
+        *paths = g_slist_prepend(*paths, g_strdup(cur->data));
     }
 
     return 0;
@@ -1459,7 +1459,7 @@ qemuNamespaceUnlinkPaths(virDomainObj *vm,
         if (STRPREFIX(path, QEMU_DEVPREFIX)) {
             GStrv mount;
             bool inSubmount = false;
-            const char *const *devices = (const char *const 
*)cfg->cgroupDeviceACL;
+            GSList *devices = cfg->cgroupDeviceACL;
 
             for (mount = devMountsPath; *mount; mount++) {
                 if (STREQ(*mount, "/dev"))
@@ -1477,7 +1477,7 @@ qemuNamespaceUnlinkPaths(virDomainObj *vm,
             if (!devices)
                 devices = defaultDeviceACL;
 
-            if (g_strv_contains(devices, path))
+            if (g_slist_find(devices, path))
                 continue;
 
             unlinkPaths = g_slist_prepend(unlinkPaths, g_strdup(path));
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9926998f85..d3a78266ef 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3038,7 +3038,7 @@ qemuProcessAllowPostCopyMigration(virDomainObj *vm)
     qemuDomainObjPrivate *priv = vm->privateData;
     virQEMUDriver *driver = priv->driver;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-    const char *const *devices = (const char *const *) cfg->cgroupDeviceACL;
+    GSList *devices = cfg->cgroupDeviceACL;
     const char *uffd = "/dev/userfaultfd";
     int rc;
 
@@ -3050,7 +3050,7 @@ qemuProcessAllowPostCopyMigration(virDomainObj *vm)
     if (!devices)
         devices = defaultDeviceACL;
 
-    if (!g_strv_contains(devices, uffd)) {
+    if (!g_slist_find(devices, uffd)) {
         VIR_DEBUG("%s is not allowed by device ACL", uffd);
         return 0;
     }
@@ -8193,6 +8193,8 @@ qemuProcessLaunch(virConnectPtr conn,
         goto cleanup;
     }
 
+    updateDefaultDeviceACL(vm);
+
     VIR_DEBUG("Building domain mount namespace (if required)");
     if (qemuDomainBuildNamespace(cfg, vm) < 0)
         goto cleanup;
diff --git a/src/qemu/test_libvirtd_qemu.aug.in 
b/src/qemu/test_libvirtd_qemu.aug.in
index 90012b3f52..82cfec3b4b 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -76,8 +76,7 @@ module Test_libvirtd_qemu =
     { "4" = "/dev/random" }
     { "5" = "/dev/urandom" }
     { "6" = "/dev/ptmx" }
-    { "7" = "/dev/kvm" }
-    { "8" = "/dev/userfaultfd" }
+    { "7" = "/dev/userfaultfd" }
 }
 { "save_image_format" = "raw" }
 { "dump_image_format" = "raw" }
-- 
2.51.0

Reply via email to