On Tue, Jan 06, 2026 at 06:49:37PM -0800, Nathan Chen via Devel wrote:
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 | 32 +++++++++++++----
src/security/security_dac.c | 59 ++++++++++++++++++++++++++------
src/security/security_selinux.c | 57 ++++++++++++++++++++++++------
src/security/virt-aa-helper.c | 33 ++++++++++++++----
6 files changed, 170 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;
I would change the code like this:
if (dev->source.subsys.u.pci.driver.iommufd == VIR_TRISTATE_BOOL_YES)
return 0;
...
That way you don't have to move the whole code into the if condition.
This applies to changes in qemu_namespace.c as well.
- 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..362ca09562 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -848,14 +848,32 @@ 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;
+
+ if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr,
&vfiofdDev) < 0)
+ return -1;
+
+ if (!virIOMMUFDSupported())
+ return -1;
Move this check before we try to get vfio path as there is no need to
construct the path if iommufd is not supported. We should also report
error here, if virIOMMUFDSupported() fails it only sets errno.
User goto done; instead of return -1; other we are going to leak ptr
and pci.
}
- 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..fbe216637f 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -41,6 +41,7 @@
#include "virscsivhost.h"
#include "virstring.h"
#include "virutil.h"
+#include "viriommufd.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -1282,14 +1283,32 @@ 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;
+
+ if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr,
&vfiofdDev) < 0)
+ return -1;
- ret = virSecurityDACSetHostdevLabelHelper(vfioGroupDev,
- false,
- &cbdata);
+ if (!virIOMMUFDSupported())
+ return -1;
Same as in apparmor, move this check before virPCIDeviceGetVfioPath()
and report error.
+
+ ret = virSecurityDACSetHostdevLabelHelper(vfiofdDev, false,
&cbdata);
+ if (ret)
+ return ret;
+
+ ret = virSecurityDACSetHostdevLabelHelper(VIR_IOMMU_DEV_PATH,
false, &cbdata);
+ if (ret)
+ return ret;
+ }
} else {
ret = virPCIDeviceFileIterate(pci,
virSecurityDACSetPCILabel,
@@ -1443,13 +1462,33 @@ 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;
+
+ if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr,
&vfiofdDev) < 0)
+ return -1;
+
+ if (!virIOMMUFDSupported())
+ return -1;
Same here.
+
+ ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL,
+ vfiofdDev, false);
+ if (ret)
+ return ret;
+
+ ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL,
+
VIR_IOMMU_DEV_PATH, 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..05086ad9e1 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -41,6 +41,7 @@
#include "virconf.h"
#include "virtpm.h"
#include "virstring.h"
+#include "viriommufd.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -2256,14 +2257,32 @@ 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;
+
+ if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr,
&vfiofdDev) < 0)
+ return -1;
- ret = virSecuritySELinuxSetHostdevLabelHelper(vfioGroupDev,
- false,
- &data);
+ if (!virIOMMUFDSupported())
+ return -1;
Same here.
+
+ ret = virSecuritySELinuxSetHostdevLabelHelper(vfiofdDev, false,
&data);
+ if (ret)
+ return ret;
+
+ ret = virSecuritySELinuxSetHostdevLabelHelper(VIR_IOMMU_DEV_PATH,
false, &data);
+ if (ret)
+ return ret;
+ }
} else {
ret = virPCIDeviceFileIterate(pci, virSecuritySELinuxSetPCILabel,
&data);
}
@@ -2491,12 +2510,30 @@
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;
+
+ if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr,
&vfiofdDev) < 0)
+ return -1;
- ret = virSecuritySELinuxRestoreFileLabel(mgr, vfioGroupDev, false, false);
+ if (!virIOMMUFDSupported())
+ return -1;
Same here.
+
+ ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiofdDev,
false, false);
+ if (ret)
+ return ret;
+
+ ret = virSecuritySELinuxRestoreFileLabel(mgr,
VIR_IOMMU_DEV_PATH, false, false);
+ if (ret)
+ return ret;
+ }
} else {
ret = virPCIDeviceFileIterate(pci,
virSecuritySELinuxRestorePCILabel, mgr);
}
Pavel
