[PATCH 13/14] Avoid unsafe use of /proc/$PID/root in LXC chardev hostdev hotplug

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list