[libvirt] [PATCH 07/10] qemuDomainCreateDevice: Properly deal with symlinks

Michal Privoznik mprivozn at redhat.com
Fri Jan 20 09:42:47 UTC 2017


Imagine you have a disk with the following source set up:

/dev/disk/by-uuid/$uuid (symlink to) -> /dev/sda

After cbc45525cb21 the transitive end of the symlink chain is
created (/dev/sda), but we need to create any item in chain too.
Others might rely on that.
In this case, /dev/disk/by-uuid/$uuid comes from domain XML thus
it is this path that secdriver tries to relabel. Not the resolved
one.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/qemu/qemu_domain.c | 150 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 108 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0f45f753e..8cbfb2d16 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -68,6 +68,7 @@
 #endif
 
 #include <libxml/xpathInternals.h>
+#include "dosname.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -6958,70 +6959,135 @@ qemuDomainCreateDevice(const char *device,
                        bool allow_noent)
 {
     char *devicePath = NULL;
-    char *canonDevicePath = NULL;
+    char *target = NULL;
     struct stat sb;
     int ret = -1;
+    bool isLink = false;
+    bool create = false;
 #ifdef WITH_SELINUX
     char *tcon = NULL;
 #endif
 
-    if (virFileResolveAllLinks(device, &canonDevicePath) < 0) {
+    if (lstat(device, &sb) < 0) {
         if (errno == ENOENT && allow_noent) {
             /* Ignore non-existent device. */
-            ret = 0;
-            goto cleanup;
+            return 0;
         }
-
-        virReportError(errno, _("Unable to canonicalize %s"), device);
-        goto cleanup;
-    }
-
-    if (!STRPREFIX(canonDevicePath, DEVPREFIX)) {
-        ret = 0;
-        goto cleanup;
+        virReportSystemError(errno, _("Unable to stat %s"), device);
+        return ret;
     }
 
-    if (virAsprintf(&devicePath, "%s/%s",
-                    path, canonDevicePath + strlen(DEVPREFIX)) < 0)
-        goto cleanup;
+    isLink = S_ISLNK(sb.st_mode);
 
-    if (stat(canonDevicePath, &sb) < 0) {
-        if (errno == ENOENT && allow_noent) {
-            /* Ignore non-existent device. */
-            ret = 0;
+    /* Here, @device might be whatever path in the system. We
+     * should create the path in the namespace iff its "/dev"
+     * prefixed. However, if it is a symlink, we need to traverse
+     * it too (it might point to something in "/dev"). Just
+     * consider:
+     *
+     *   /var/sym1 -> /var/sym2 -> /dev/sda  (because users can)
+     *
+     * This means, "/var/sym1" is not created (it's shared with
+     * the parent namespace), nor "/var/sym2", but "/dev/sda".
+     *
+     * TODO Remove all `.' and `..' from the @device path.
+     * Otherwise we might get fooled with `/dev/../var/my_image'.
+     * For now, lets hope callers play nice.
+     */
+    if (STRPREFIX(device, DEVPREFIX)) {
+        if (virAsprintf(&devicePath, "%s/%s",
+                        path, device + strlen(DEVPREFIX)) < 0)
             goto cleanup;
-        }
-
-        virReportSystemError(errno, _("Unable to stat %s"), canonDevicePath);
-        goto cleanup;
-    }
-
-    if (virFileMakeParentPath(devicePath) < 0) {
-        virReportSystemError(errno,
-                             _("Unable to create %s"),
-                             devicePath);
-        goto cleanup;
-    }
 
-    if (mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) {
-        if (errno == EEXIST) {
-            ret = 0;
-        } else {
+        if (virFileMakeParentPath(devicePath) < 0) {
             virReportSystemError(errno,
-                                 _("Failed to make device %s"),
+                                 _("Unable to create %s"),
                                  devicePath);
+            goto cleanup;
         }
+        create = true;
+    }
+
+    if (isLink) {
+        /* We are dealing with a symlink. Create a dangling symlink and descend
+         * down one level which hopefully creates the symlink's target. */
+        if (virFileReadLink(device, &target) < 0) {
+            virReportSystemError(errno,
+                                 _("unable to resolve symlink %s"),
+                                 device);
+            goto cleanup;
+        }
+
+        if (create &&
+            symlink(target, devicePath) < 0) {
+            if (errno == EEXIST) {
+                ret = 0;
+            } else {
+                virReportSystemError(errno,
+                                     _("unable to create symlink %s"),
+                                     devicePath);
+            }
+            goto cleanup;
+        }
+
+        /* Tricky part. If the target starts with a slash then we need to take
+         * it as it is. Otherwise we need to replace the last component in the
+         * original path with the link target:
+         * /dev/rtc -> rtc0 (want /dev/rtc0)
+         * /dev/disk/by-id/ata-SanDisk_SDSSDXPS480G_161101402485 -> ../../sda
+         *   (want /dev/disk/by-id/../../sda)
+         * /dev/stdout -> /proc/self/fd/1 (no change needed)
+         */
+        if (IS_RELATIVE_FILE_NAME(target)) {
+            char *c = NULL, *tmp = NULL, *devTmp = NULL;
+
+            if (VIR_STRDUP(devTmp, device) < 0)
+                goto cleanup;
+
+            if ((c = strrchr(devTmp, '/')))
+                *(c + 1) = '\0';
+
+            if (virAsprintf(&tmp, "%s%s", devTmp, target) < 0) {
+                VIR_FREE(devTmp);
+                goto cleanup;
+            }
+            VIR_FREE(devTmp);
+            VIR_FREE(target);
+            target = tmp;
+            tmp = NULL;
+        }
+
+        if (qemuDomainCreateDevice(target, path, allow_noent) < 0)
+            goto cleanup;
+    } else {
+        if (create &&
+            mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) {
+            if (errno == EEXIST) {
+                ret = 0;
+            } else {
+                virReportSystemError(errno,
+                                     _("Failed to make device %s"),
+                                     devicePath);
+            }
+            goto cleanup;
+        }
+    }
+
+    if (!create) {
+        ret = 0;
         goto cleanup;
     }
 
-    if (chown(devicePath, sb.st_uid, sb.st_gid) < 0) {
+    if (lchown(devicePath, sb.st_uid, sb.st_gid) < 0) {
         virReportSystemError(errno,
                              _("Failed to chown device %s"),
                              devicePath);
         goto cleanup;
     }
 
-    if (virFileCopyACLs(canonDevicePath, devicePath) < 0 &&
+    /* Symlinks don't have ACLs. */
+    if (!isLink &&
+        virFileCopyACLs(device, devicePath) < 0 &&
         errno != ENOTSUP) {
         virReportSystemError(errno,
                              _("Failed to copy ACLs on device %s"),
@@ -7030,16 +7096,16 @@ qemuDomainCreateDevice(const char *device,
     }
 
 #ifdef WITH_SELINUX
-    if (getfilecon_raw(canonDevicePath, &tcon) < 0 &&
+    if (lgetfilecon_raw(device, &tcon) < 0 &&
         (errno != ENOTSUP && errno != ENODATA)) {
         virReportSystemError(errno,
                              _("Unable to get SELinux label from %s"),
-                             canonDevicePath);
+                             device);
         goto cleanup;
     }
 
     if (tcon &&
-        setfilecon_raw(devicePath, (VIR_SELINUX_CTX_CONST char *) tcon) < 0) {
+        lsetfilecon_raw(devicePath, (VIR_SELINUX_CTX_CONST char *) tcon) < 0) {
         VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
         if (errno != EOPNOTSUPP && errno != ENOTSUP) {
         VIR_WARNINGS_RESET
@@ -7053,7 +7119,7 @@ qemuDomainCreateDevice(const char *device,
 
     ret = 0;
  cleanup:
-    VIR_FREE(canonDevicePath);
+    VIR_FREE(target);
     VIR_FREE(devicePath);
 #ifdef WITH_SELINUX
     freecon(tcon);
-- 
2.11.0




More information about the libvir-list mailing list