From: Daniel P. Berrange <berrange@redhat.com>
Date: Thu, 30 Jan 2014 17:58:36 +0000 (+0000)
Subject: CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC hotunplug code
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=61c7e0b66e8b37d4ea64024c100d2ed467d5cb47;p=thirdparty%2Flibvirt.git

CVE-2013-6456: Avoid unsafe use of /proc/$PID/root in LXC hotunplug code

Rewrite multiple hotunplug functions to to use the
virProcessRunInMountNamespace helper. This avoids
risk of a malicious guest replacing /dev with an absolute
symlink, tricking the driver into changing the host OS
filesystem.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 5fc590ad9f4071350a8df4d567ba88baacc8334d)

Conflicts:
	src/lxc/lxc_driver.c: OOM + cgroups error reporting
---

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index ca314a025c..f4f5a84514 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3378,6 +3378,39 @@ lxcDomainAttachDeviceMknod(virLXCDriverPtr driver,
 }
 
 
+static int
+lxcDomainAttachDeviceUnlinkHelper(pid_t pid ATTRIBUTE_UNUSED,
+                                  void *opaque)
+{
+    const char *path = opaque;
+
+    VIR_DEBUG("Unlinking %s", path);
+    if (unlink(path) < 0 && errno != ENOENT) {
+        virReportSystemError(errno,
+                             _("Unable to remove device %s"), path);
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
+lxcDomainAttachDeviceUnlink(virDomainObjPtr vm,
+                            char *file)
+{
+    virLXCDomainObjPrivatePtr priv = vm->privateData;
+
+    if (virProcessRunInMountNamespace(priv->initpid,
+                                      lxcDomainAttachDeviceUnlinkHelper,
+                                      file) < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver,
                               virDomainObjPtr vm,
@@ -3992,8 +4025,7 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm,
 
     def = vm->def->disks[i];
 
-    if (virAsprintf(&dst, "/proc/%llu/root/dev/%s",
-                    (unsigned long long)priv->initpid, def->dst) < 0) {
+    if (virAsprintf(&dst, "/dev/%s", def->dst) < 0) {
         virReportOOMError();
         goto cleanup;
     }
@@ -4004,11 +4036,8 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm,
         goto cleanup;
     }
 
-    VIR_DEBUG("Unlinking %s (backed by %s)", dst, def->src);
-    if (unlink(dst) < 0 && errno != ENOENT) {
+    if (lxcDomainAttachDeviceUnlink(vm, dst) < 0) {
         virDomainAuditDisk(vm, def->src, NULL, "detach", false);
-        virReportSystemError(errno,
-                             _("Unable to remove device %s"), dst);
         goto cleanup;
     }
     virDomainAuditDisk(vm, def->src, NULL, "detach", true);
@@ -4103,7 +4132,6 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,
     virDomainHostdevDefPtr def = NULL;
     int idx, ret = -1;
     char *dst = NULL;
-    char *vroot = NULL;
     virUSBDevicePtr usb = NULL;
 
     if ((idx = virDomainHostdevFind(vm->def,
@@ -4114,14 +4142,7 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,
         goto cleanup;
     }
 
-    if (virAsprintf(&vroot, "/proc/%llu/root",
-                    (unsigned long long)priv->initpid) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
-    if (virAsprintf(&dst, "%s/dev/bus/usb/%03d/%03d",
-                    vroot,
+    if (virAsprintf(&dst, "/dev/bus/usb/%03d/%03d",
                     def->source.subsys.u.usb.bus,
                     def->source.subsys.u.usb.device) < 0) {
         virReportOOMError();
@@ -4138,11 +4159,8 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,
                                 def->source.subsys.u.usb.device, NULL)))
         goto cleanup;
 
-    VIR_DEBUG("Unlinking %s", dst);
-    if (unlink(dst) < 0 && errno != ENOENT) {
+    if (lxcDomainAttachDeviceUnlink(vm, dst) < 0) {
         virDomainAuditHostdev(vm, def, "detach", false);
-        virReportSystemError(errno,
-                             _("Unable to remove device %s"), dst);
         goto cleanup;
     }
     virDomainAuditHostdev(vm, def, "detach", true);
@@ -4163,7 +4181,6 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,
 cleanup:
     virUSBDeviceFree(usb);
     VIR_FREE(dst);
-    VIR_FREE(vroot);
     return ret;
 }
 
@@ -4175,7 +4192,6 @@ lxcDomainDetachDeviceHostdevStorageLive(virDomainObjPtr vm,
     virLXCDomainObjPrivatePtr priv = vm->privateData;
     virDomainHostdevDefPtr def = NULL;
     int i, ret = -1;
-    char *dst = NULL;
 
     if (!priv->initpid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -4192,24 +4208,14 @@ lxcDomainDetachDeviceHostdevStorageLive(virDomainObjPtr vm,
         goto cleanup;
     }
 
-    if (virAsprintf(&dst, "/proc/%llu/root/%s",
-                    (unsigned long long)priv->initpid,
-                    def->source.caps.u.storage.block) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("devices cgroup isn't mounted"));
         goto cleanup;
     }
 
-    VIR_DEBUG("Unlinking %s", dst);
-    if (unlink(dst) < 0 && errno != ENOENT) {
+    if (lxcDomainAttachDeviceUnlink(vm, def->source.caps.u.storage.block) < 0) {
         virDomainAuditHostdev(vm, def, "detach", false);
-        virReportSystemError(errno,
-                             _("Unable to remove device %s"), dst);
         goto cleanup;
     }
     virDomainAuditHostdev(vm, def, "detach", true);
@@ -4224,7 +4230,6 @@ lxcDomainDetachDeviceHostdevStorageLive(virDomainObjPtr vm,
     ret = 0;
 
 cleanup:
-    VIR_FREE(dst);
     return ret;
 }
 
@@ -4236,7 +4241,6 @@ lxcDomainDetachDeviceHostdevMiscLive(virDomainObjPtr vm,
     virLXCDomainObjPrivatePtr priv = vm->privateData;
     virDomainHostdevDefPtr def = NULL;
     int i, ret = -1;
-    char *dst = NULL;
 
     if (!priv->initpid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -4253,24 +4257,14 @@ lxcDomainDetachDeviceHostdevMiscLive(virDomainObjPtr vm,
         goto cleanup;
     }
 
-    if (virAsprintf(&dst, "/proc/%llu/root/%s",
-                    (unsigned long long)priv->initpid,
-                    def->source.caps.u.misc.chardev) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("devices cgroup isn't mounted"));
         goto cleanup;
     }
 
-    VIR_DEBUG("Unlinking %s", dst);
-    if (unlink(dst) < 0 && errno != ENOENT) {
+    if (lxcDomainAttachDeviceUnlink(vm, def->source.caps.u.misc.chardev) < 0) {
         virDomainAuditHostdev(vm, def, "detach", false);
-        virReportSystemError(errno,
-                             _("Unable to remove device %s"), dst);
         goto cleanup;
     }
     virDomainAuditHostdev(vm, def, "detach", true);
@@ -4285,7 +4279,6 @@ lxcDomainDetachDeviceHostdevMiscLive(virDomainObjPtr vm,
     ret = 0;
 
 cleanup:
-    VIR_FREE(dst);
     return ret;
 }