[libvirt] [PATCH 13/14] Avoid unsafe use of /proc/$PID/root in LXC chardev hostdev hotplug
Rewrite lxcDomainAttachDeviceHostdevMisceLive function
to use the virProcessRunInMountNamespace helper. This avoids
risk of a malicious guest replacing /dev with a absolute
symlink, tricking the driver into changing the host OS
filesystem.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_driver.c | 66 ++++++++++++++--------------------------------------
1 file changed, 18 insertions(+), 48 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 5780e11..763b22b 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4130,11 +4130,7 @@ lxcDomainAttachDeviceHostdevMiscLive(virLXCDriverPtr driver,
virLXCDomainObjPrivatePtr priv = vm->privateData;
virDomainHostdevDefPtr def = dev->data.hostdev;
int ret = -1;
- char *dst = NULL;
- char *vroot = NULL;
struct stat sb;
- bool created = false;
- mode_t mode = 0;
if (!def->source.caps.u.misc.chardev) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -4162,51 +4158,29 @@ lxcDomainAttachDeviceHostdevMiscLive(virLXCDriverPtr driver,
goto cleanup;
}
- if (virAsprintf(&vroot, "/proc/%llu/root",
- (unsigned long long)priv->initpid) < 0)
- goto cleanup;
-
- if (virAsprintf(&dst, "%s/%s",
- vroot,
- def->source.caps.u.misc.chardev) < 0)
- goto cleanup;
-
- if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0)
- goto cleanup;
-
- if (lxcContainerSetupHostdevCapsMakePath(dst) < 0) {
- virReportSystemError(errno,
- _("Unable to create directory for device %s"),
- dst);
- goto cleanup;
- }
-
- mode = 0700 | S_IFCHR;
-
- VIR_DEBUG("Creating dev %s (%d,%d)",
- def->source.caps.u.misc.chardev,
- major(sb.st_rdev), minor(sb.st_rdev));
- if (mknod(dst, mode, sb.st_rdev) < 0) {
- virReportSystemError(errno,
- _("Unable to create device %s"),
- dst);
+ if (virCgroupAllowDevicePath(priv->cgroup,
+ def->source.caps.u.misc.chardev,
+ VIR_CGROUP_DEVICE_RWM) != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot allow device %s for domain %s"),
+ def->source.caps.u.misc.chardev, vm->def->name);
goto cleanup;
}
- created = true;
-
- if (lxcContainerChown(vm->def, dst) < 0)
- goto cleanup;
- if (virSecurityManagerSetHostdevLabel(driver->securityManager,
- vm->def, def, vroot) < 0)
+ if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0)
goto cleanup;
- if (virCgroupAllowDevicePath(priv->cgroup, def->source.caps.u.misc.chardev,
- VIR_CGROUP_DEVICE_RW |
- VIR_CGROUP_DEVICE_MKNOD) != 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot allow device %s for domain %s"),
- def->source.caps.u.misc.chardev, vm->def->name);
+ if (lxcDomainAttachDeviceMknod(driver,
+ 0700 | S_IFBLK,
+ sb.st_rdev,
+ vm,
+ dev,
+ def->source.caps.u.misc.chardev) < 0) {
+ if (virCgroupDenyDevicePath(priv->cgroup,
+ def->source.caps.u.misc.chardev,
+ VIR_CGROUP_DEVICE_RWM) != 0)
+ VIR_WARN("cannot deny device %s for domain %s",
+ def->source.caps.u.storage.block, vm->def->name);
goto cleanup;
}
@@ -4216,10 +4190,6 @@ lxcDomainAttachDeviceHostdevMiscLive(virLXCDriverPtr driver,
cleanup:
virDomainAuditHostdev(vm, def, "attach", ret == 0);
- if (dst && created && ret < 0)
- unlink(dst);
- VIR_FREE(dst);
- VIR_FREE(vroot);
return ret;
}
--
1.8.5.3