From f84056cf6166332b1f15f3e6584a88f5d42273fe Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Thu, 30 Jan 2014 17:58:36 +0000 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) Conflicts: src/lxc/lxc_driver.c: OOM + cgroups error reporting --- src/lxc/lxc_driver.c | 85 ++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 46 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index faf9294c52..eb7801e09e 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3229,6 +3229,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, @@ -3843,8 +3876,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; } @@ -3855,11 +3887,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); @@ -3954,7 +3983,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, @@ -3965,14 +3993,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(); @@ -3989,11 +4010,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); @@ -4014,7 +4032,6 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, cleanup: virUSBDeviceFree(usb); VIR_FREE(dst); - VIR_FREE(vroot); return ret; } @@ -4026,7 +4043,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", @@ -4043,24 +4059,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); @@ -4075,7 +4081,6 @@ lxcDomainDetachDeviceHostdevStorageLive(virDomainObjPtr vm, ret = 0; cleanup: - VIR_FREE(dst); return ret; } @@ -4087,7 +4092,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", @@ -4104,24 +4108,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); @@ -4136,7 +4130,6 @@ lxcDomainDetachDeviceHostdevMiscLive(virDomainObjPtr vm, ret = 0; cleanup: - VIR_FREE(dst); return ret; }