From: Nathan Chen <[email protected]> When launching a qemu VM with the iommufd feature enabled for VFIO hostdevs: - Do not allow cgroup, namespace, and seclabel access to VFIO paths (/dev/vfio/vfio and /dev/vfio/<iommugroup>) - Allow access to iommufd paths (/dev/iommu and /dev/vfio/devices/vfio*) for AppArmor, SELinux, and DAC
Signed-off-by: Nathan Chen <[email protected]> --- src/qemu/qemu_cgroup.c | 26 +++++++------- src/qemu/qemu_namespace.c | 16 +++++---- src/security/security_apparmor.c | 33 ++++++++++++++---- src/security/security_dac.c | 60 ++++++++++++++++++++++++++------ src/security/security_selinux.c | 58 ++++++++++++++++++++++++------ src/security/virt-aa-helper.c | 32 +++++++++++++---- 6 files changed, 172 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 7dadef0739..7190a4f80f 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -479,21 +479,23 @@ qemuSetupHostdevCgroup(virDomainObj *vm, g_autofree char *path = NULL; int perms; - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) - return 0; + if (dev->source.subsys.u.pci.driver.iommufd != VIR_TRISTATE_BOOL_YES) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) + return 0; - if (qemuDomainGetHostdevPath(dev, &path, &perms) < 0) - return -1; + if (qemuDomainGetHostdevPath(dev, &path, &perms) < 0) + return -1; - if (path && - qemuCgroupAllowDevicePath(vm, path, perms, false) < 0) { - return -1; - } + if (path && + qemuCgroupAllowDevicePath(vm, path, perms, false) < 0) { + return -1; + } - if (virHostdevNeedsVFIO(dev) && - qemuCgroupAllowDevicePath(vm, QEMU_DEV_VFIO, - VIR_CGROUP_DEVICE_RW, false) < 0) { - return -1; + if (virHostdevNeedsVFIO(dev) && + qemuCgroupAllowDevicePath(vm, QEMU_DEV_VFIO, + VIR_CGROUP_DEVICE_RW, false) < 0) { + return -1; + } } return 0; diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index c689cc3e40..907b2773cf 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -345,15 +345,17 @@ qemuDomainSetupHostdev(virDomainObj *vm, { g_autofree char *path = NULL; - if (qemuDomainGetHostdevPath(hostdev, &path, NULL) < 0) - return -1; + if (hostdev->source.subsys.u.pci.driver.iommufd != VIR_TRISTATE_BOOL_YES) { + if (qemuDomainGetHostdevPath(hostdev, &path, NULL) < 0) + return -1; - if (path) - *paths = g_slist_prepend(*paths, g_steal_pointer(&path)); + if (path) + *paths = g_slist_prepend(*paths, g_steal_pointer(&path)); - if (virHostdevNeedsVFIO(hostdev) && - (!hotplug || !qemuDomainNeedsVFIO(vm->def))) - *paths = g_slist_prepend(*paths, g_strdup(QEMU_DEV_VFIO)); + if (virHostdevNeedsVFIO(hostdev) && + (!hotplug || !qemuDomainNeedsVFIO(vm->def))) + *paths = g_slist_prepend(*paths, g_strdup(QEMU_DEV_VFIO)); + } return 0; } diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 68ac39611f..999275dac1 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -848,14 +848,33 @@ AppArmorSetSecurityHostdevLabel(virSecurityManager *mgr, goto done; if (pcisrc->driver.name == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO) { - char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); - - if (!vfioGroupDev) { - virPCIDeviceFree(pci); - goto done; + if (dev->source.subsys.u.pci.driver.iommufd != VIR_TRISTATE_BOOL_YES) { + char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); + + if (!vfioGroupDev) { + virPCIDeviceFree(pci); + goto done; + } + ret = AppArmorSetSecurityPCILabel(pci, vfioGroupDev, ptr); + VIR_FREE(vfioGroupDev); + } else { + g_autofree char *vfiofdDev = NULL; + const char *iommufdDir = "/dev/iommu"; + + if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr, &vfiofdDev) < 0) + return -1; + + if (!virFileExists(iommufdDir)) + return -1; + + ret = AppArmorSetSecurityPCILabel(pci, vfiofdDev, ptr); + if (ret) + return ret; + + ret = AppArmorSetSecurityPCILabel(pci, iommufdDir, ptr); + if (ret) + return ret; } - ret = AppArmorSetSecurityPCILabel(pci, vfioGroupDev, ptr); - VIR_FREE(vfioGroupDev); } else { ret = virPCIDeviceFileIterate(pci, AppArmorSetSecurityPCILabel, ptr); } diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 2f788b872a..09e26033ac 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1282,14 +1282,33 @@ virSecurityDACSetHostdevLabel(virSecurityManager *mgr, return -1; if (pcisrc->driver.name == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO) { - g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); + if (dev->source.subsys.u.pci.driver.iommufd != VIR_TRISTATE_BOOL_YES) { + g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); - if (!vfioGroupDev) - return -1; + if (!vfioGroupDev) + return -1; + + ret = virSecurityDACSetHostdevLabelHelper(vfioGroupDev, + false, + &cbdata); + } else { + g_autofree char *vfiofdDev = NULL; + const char *iommufdDir = "/dev/iommu"; + + if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr, &vfiofdDev) < 0) + return -1; - ret = virSecurityDACSetHostdevLabelHelper(vfioGroupDev, - false, - &cbdata); + if (!virFileExists(iommufdDir)) + return -1; + + ret = virSecurityDACSetHostdevLabelHelper(vfiofdDev, false, &cbdata); + if (ret) + return ret; + + ret = virSecurityDACSetHostdevLabelHelper(iommufdDir, false, &cbdata); + if (ret) + return ret; + } } else { ret = virPCIDeviceFileIterate(pci, virSecurityDACSetPCILabel, @@ -1443,13 +1462,34 @@ virSecurityDACRestoreHostdevLabel(virSecurityManager *mgr, return -1; if (pcisrc->driver.name == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO) { - g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); + if (dev->source.subsys.u.pci.driver.iommufd != VIR_TRISTATE_BOOL_YES) { + g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); - if (!vfioGroupDev) - return -1; + if (!vfioGroupDev) + return -1; - ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL, + ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL, vfioGroupDev, false); + } else { + g_autofree char *vfiofdDev = NULL; + const char *iommufdDir = "/dev/iommu"; + + if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr, &vfiofdDev) < 0) + return -1; + + if (!virFileExists(iommufdDir)) + return -1; + + ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL, + vfiofdDev, false); + if (ret) + return ret; + + ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL, + iommufdDir, false); + if (ret) + return ret; + } } else { ret = virPCIDeviceFileIterate(pci, virSecurityDACRestorePCILabel, mgr); } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 2f3cc274a5..1dd0a9706a 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2256,14 +2256,33 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManager *mgr, return -1; if (pcisrc->driver.name == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO) { - g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); + if (dev->source.subsys.u.pci.driver.iommufd != VIR_TRISTATE_BOOL_YES) { + g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); - if (!vfioGroupDev) - return -1; + if (!vfioGroupDev) + return -1; + + ret = virSecuritySELinuxSetHostdevLabelHelper(vfioGroupDev, + false, + &data); + } else { + g_autofree char *vfiofdDev = NULL; + const char *iommufdDir = "/dev/iommu"; + + if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr, &vfiofdDev) < 0) + return -1; - ret = virSecuritySELinuxSetHostdevLabelHelper(vfioGroupDev, - false, - &data); + if (!virFileExists(iommufdDir)) + return -1; + + ret = virSecuritySELinuxSetHostdevLabelHelper(vfiofdDev, false, &data); + if (ret) + return ret; + + ret = virSecuritySELinuxSetHostdevLabelHelper(iommufdDir, false, &data); + if (ret) + return ret; + } } else { ret = virPCIDeviceFileIterate(pci, virSecuritySELinuxSetPCILabel, &data); } @@ -2491,12 +2510,31 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManager *mgr, return -1; if (pcisrc->driver.name == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO) { - g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); + if (dev->source.subsys.u.pci.driver.iommufd != VIR_TRISTATE_BOOL_YES) { + g_autofree char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); - if (!vfioGroupDev) - return -1; + if (!vfioGroupDev) + return -1; + + ret = virSecuritySELinuxRestoreFileLabel(mgr, vfioGroupDev, false, false); + } else { + g_autofree char *vfiofdDev = NULL; + const char *iommufdDir = "/dev/iommu"; + + if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr, &vfiofdDev) < 0) + return -1; - ret = virSecuritySELinuxRestoreFileLabel(mgr, vfioGroupDev, false, false); + if (!virFileExists(iommufdDir)) + return -1; + + ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiofdDev, false, false); + if (ret) + return ret; + + ret = virSecuritySELinuxRestoreFileLabel(mgr, iommufdDir, false, false); + if (ret) + return ret; + } } else { ret = virPCIDeviceFileIterate(pci, virSecuritySELinuxRestorePCILabel, mgr); } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index de0a826063..5b320fbc89 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1114,8 +1114,9 @@ get_files(vahControl * ctl) virDeviceHostdevPCIDriverName driverName = dev->source.subsys.u.pci.driver.name; - if (driverName == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO || - driverName == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_DEFAULT) { + if ((driverName == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO || + driverName == VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_DEFAULT) && + dev->source.subsys.u.pci.driver.iommufd != VIR_TRISTATE_BOOL_YES) { needsVfio = true; } @@ -1348,6 +1349,7 @@ get_files(vahControl * ctl) virBufferAddLit(&buf, " \"/dev/vfio/vfio\" rw,\n"); virBufferAddLit(&buf, " \"/dev/vfio/[0-9]*\" rw,\n"); } + if (needsgl) { /* if using gl all sorts of further dri related paths will be needed */ virBufferAddLit(&buf, " # DRI/Mesa/(e)GL config and driver paths\n"); @@ -1385,9 +1387,18 @@ get_files(vahControl * ctl) } } - if (ctl->newfile && - vah_add_file(&buf, ctl->newfile, "rwk") != 0) { - return -1; + if (ctl->newfile) { + const char *perms = "rwk"; + + /* VFIO and iommufd devices need mmap permission */ + if (STRPREFIX(ctl->newfile, "/dev/vfio/devices/vfio") || + STREQ(ctl->newfile, "/dev/iommu")) { + perms = "rwm"; + } + + if (vah_add_file(&buf, ctl->newfile, perms) != 0) { + return -1; + } } ctl->files = virBufferContentAndReset(&buf); @@ -1561,8 +1572,15 @@ main(int argc, char **argv) } } if (ctl->append && ctl->newfile) { - if (vah_add_file(&buf, ctl->newfile, "rwk") != 0) - goto cleanup; + const char *perms = "rwk"; + + if (STRPREFIX(ctl->newfile, "/dev/vfio/devices/vfio") || + STREQ(ctl->newfile, "/dev/iommu")) { + perms = "rwm"; + } + + if (vah_add_file(&buf, ctl->newfile, perms) != 0) + return -1; } else { if (ctl->def->virtType == VIR_DOMAIN_VIRT_QEMU || ctl->def->virtType == VIR_DOMAIN_VIRT_KQEMU || -- 2.43.0
