From ca5a9fee089184bd26e1c0daefd3802c50f120c7 Mon Sep 17 00:00:00 2001 Message-Id: From: "Daniel P. Berrange" 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 (cherry picked from commit 5fc590ad9f4071350a8df4d567ba88baacc8334d) Signed-off-by: Jiri Denemark --- 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