147b37
From 2a7678f0966137f6e9b226934ab8dd67d6f2e963 Mon Sep 17 00:00:00 2001
147b37
Message-Id: <2a7678f0966137f6e9b226934ab8dd67d6f2e963@dist-git>
147b37
From: Michal Privoznik <mprivozn@redhat.com>
147b37
Date: Mon, 9 Apr 2018 15:46:50 +0200
147b37
Subject: [PATCH] qemu_cgroup: Handle device mapper targets properly
147b37
MIME-Version: 1.0
147b37
Content-Type: text/plain; charset=UTF-8
147b37
Content-Transfer-Encoding: 8bit
147b37
147b37
RHEL-7.6: https://bugzilla.redhat.com/show_bug.cgi?id=1557769
147b37
RHEL-7.5.z: https://bugzilla.redhat.com/show_bug.cgi?id=1564996
147b37
147b37
Problem with device mapper targets is that there can be several
147b37
other devices 'hidden' behind them. For instance, /dev/dm-1 can
147b37
consist of /dev/sda, /dev/sdb and /dev/sdc. Therefore, when
147b37
setting up devices CGroup and namespaces we have to take this
147b37
into account.
147b37
147b37
This bug was exposed after Linux kernel was fixed. Initially,
147b37
kernel used different functions for getting block device in
147b37
open() and ioctl(). While CGroup permissions were checked in the
147b37
former case, due to a bug in kernel they were not checked in the
147b37
latter case. This changed with the upstream commit of
147b37
519049afead4f7c3e6446028c41e99fde958cc04 (v4.16-rc5~11^2~4).
147b37
147b37
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
147b37
(cherry picked from commit 6dd84f6850ca4379203d1e7b999430ed59041208)
147b37
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
147b37
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
147b37
Reviewed-by: Ján Tomko <jtomko@redhat.com>
147b37
---
147b37
 src/qemu/qemu_cgroup.c | 46 +++++++++++++++++++++++++++++++++++++++---
147b37
 1 file changed, 43 insertions(+), 3 deletions(-)
147b37
147b37
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
147b37
index b604edb31c..d88eb7881f 100644
147b37
--- a/src/qemu/qemu_cgroup.c
147b37
+++ b/src/qemu/qemu_cgroup.c
147b37
@@ -37,6 +37,7 @@
147b37
 #include "virtypedparam.h"
147b37
 #include "virnuma.h"
147b37
 #include "virsystemd.h"
147b37
+#include "virdevmapper.h"
147b37
 
147b37
 #define VIR_FROM_THIS VIR_FROM_QEMU
147b37
 
147b37
@@ -60,7 +61,10 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
147b37
 {
147b37
     qemuDomainObjPrivatePtr priv = vm->privateData;
147b37
     int perms = VIR_CGROUP_DEVICE_READ;
147b37
-    int ret;
147b37
+    char **targetPaths = NULL;
147b37
+    size_t i;
147b37
+    int rv;
147b37
+    int ret = -1;
147b37
 
147b37
     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
147b37
         return 0;
147b37
@@ -71,12 +75,41 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
147b37
     VIR_DEBUG("Allow path %s, perms: %s",
147b37
               path, virCgroupGetDevicePermsString(perms));
147b37
 
147b37
-    ret = virCgroupAllowDevicePath(priv->cgroup, path, perms, true);
147b37
+    rv = virCgroupAllowDevicePath(priv->cgroup, path, perms, true);
147b37
 
147b37
     virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
147b37
                              virCgroupGetDevicePermsString(perms),
147b37
-                             ret);
147b37
+                             rv);
147b37
+    if (rv < 0)
147b37
+        goto cleanup;
147b37
 
147b37
+    if (rv > 0) {
147b37
+        /* @path is neither character device nor block device. */
147b37
+        ret = 0;
147b37
+        goto cleanup;
147b37
+    }
147b37
+
147b37
+    if (virDevMapperGetTargets(path, &targetPaths) < 0 &&
147b37
+        errno != ENOSYS && errno != EBADF) {
147b37
+        virReportSystemError(errno,
147b37
+                             _("Unable to get devmapper targets for %s"),
147b37
+                             path);
147b37
+        goto cleanup;
147b37
+    }
147b37
+
147b37
+    for (i = 0; targetPaths && targetPaths[i]; i++) {
147b37
+        rv = virCgroupAllowDevicePath(priv->cgroup, targetPaths[i], perms, false);
147b37
+
147b37
+        virDomainAuditCgroupPath(vm, priv->cgroup, "allow", targetPaths[i],
147b37
+                                 virCgroupGetDevicePermsString(perms),
147b37
+                                 rv);
147b37
+        if (rv < 0)
147b37
+            goto cleanup;
147b37
+    }
147b37
+
147b37
+    ret = 0;
147b37
+ cleanup:
147b37
+    virStringListFree(targetPaths);
147b37
     return ret;
147b37
 }
147b37
 
147b37
@@ -131,6 +164,13 @@ qemuTeardownImageCgroup(virDomainObjPtr vm,
147b37
     virDomainAuditCgroupPath(vm, priv->cgroup, "deny", src->path,
147b37
                              virCgroupGetDevicePermsString(perms), ret);
147b37
 
147b37
+    /* If you're looking for a counter part to
147b37
+     * qemuSetupImagePathCgroup you're at the right place.
147b37
+     * However, we can't just blindly deny all the device mapper
147b37
+     * targets of src->path because they might still be used by
147b37
+     * another disk in domain. Just like we are not removing
147b37
+     * disks from namespace. */
147b37
+
147b37
     return ret;
147b37
 }
147b37
 
147b37
-- 
147b37
2.17.0
147b37