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