From ca5a9fee089184bd26e1c0daefd3802c50f120c7 Mon Sep 17 00:00:00 2001

Message-Id: <ca5a9fee089184bd26e1c0daefd3802c50f120c7@dist-git>

From: "Daniel P. Berrange" <berrange@redhat.com>

Date: Tue, 18 Feb 2014 15:45:44 -0700

Subject: [PATCH] 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)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/lxc/lxc_driver.c | 79 ++++++++++++++++++++++++++--------------------------

 1 file changed, 39 insertions(+), 40 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c

index 91e491c..1aaa6e7 100644

--- a/src/lxc/lxc_driver.c

+++ b/src/lxc/lxc_driver.c

@@ -3179,6 +3179,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,

                               virDomainDeviceDefPtr dev)

@@ -3769,8 +3802,7 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm,

     def = vm->def->disks[idx];

-    if (virAsprintf(&dst, "/proc/%llu/root/dev/%s",

-                    (unsigned long long)priv->initpid, def->dst) < 0)

+    if (virAsprintf(&dst, "/dev/%s", def->dst) < 0)

         goto cleanup;

     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) {

@@ -3779,11 +3811,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);

@@ -3878,7 +3907,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,

@@ -3889,12 +3917,7 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,

         goto cleanup;

     }

-    if (virAsprintf(&vroot, "/proc/%llu/root",

-                    (unsigned long long)priv->initpid) < 0)

-        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)

         goto cleanup;

@@ -3909,11 +3932,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);

@@ -3936,7 +3956,6 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,

 cleanup:

     virUSBDeviceFree(usb);

     VIR_FREE(dst);

-    VIR_FREE(vroot);

     return ret;

 }

@@ -3948,7 +3967,6 @@ lxcDomainDetachDeviceHostdevStorageLive(virDomainObjPtr vm,

     virLXCDomainObjPrivatePtr priv = vm->privateData;

     virDomainHostdevDefPtr def = NULL;

     int idx, ret = -1;

-    char *dst = NULL;

     if (!priv->initpid) {

         virReportError(VIR_ERR_OPERATION_INVALID, "%s",

@@ -3965,22 +3983,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)

-        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);

@@ -3995,7 +4005,6 @@ lxcDomainDetachDeviceHostdevStorageLive(virDomainObjPtr vm,

     ret = 0;

 cleanup:

-    VIR_FREE(dst);

     return ret;

 }

@@ -4007,7 +4016,6 @@ lxcDomainDetachDeviceHostdevMiscLive(virDomainObjPtr vm,

     virLXCDomainObjPrivatePtr priv = vm->privateData;

     virDomainHostdevDefPtr def = NULL;

     int idx, ret = -1;

-    char *dst = NULL;

     if (!priv->initpid) {

         virReportError(VIR_ERR_OPERATION_INVALID, "%s",

@@ -4024,22 +4032,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)

-        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);

@@ -4054,7 +4054,6 @@ lxcDomainDetachDeviceHostdevMiscLive(virDomainObjPtr vm,

     ret = 0;

 cleanup:

-    VIR_FREE(dst);

     return ret;

 }

-- 

1.9.0