On Thu, Dec 18, 2025 at 06:19:24PM -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 | 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(-)
snip
> 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";
So we declare / use this path 11 times by the end of this series
$ git grep /dev/iommu src/
src/qemu/qemu_process.c: * Opens /dev/iommu file descriptor for the VM.
src/qemu/qemu_process.c: if ((fd = open("/dev/iommu", O_RDWR | O_CLOEXEC)) <
0) {
src/qemu/qemu_process.c: _("IOMMU FD support requires
/dev/iommu device"));
src/qemu/qemu_process.c: _("cannot open
/dev/iommu"));
src/security/security_apparmor.c: const char *iommufdDir =
"/dev/iommu";
src/security/security_dac.c: const char *iommufdDir =
"/dev/iommu";
src/security/security_dac.c: const char *iommufdDir =
"/dev/iommu";
src/security/security_selinux.c: const char *iommufdDir =
"/dev/iommu";
src/security/security_selinux.c: const char *iommufdDir =
"/dev/iommu";
src/security/virt-aa-helper.c: STREQ(ctl->newfile, "/dev/iommu")) {
src/security/virt-aa-helper.c: STREQ(ctl->newfile,
"/dev/iommu")) {
how about we declare VIR_IOMMU_DEV_PATH in src/util/viriommu.h and
use that throughout.
> +
> + if (virPCIDeviceGetVfioPath(&dev->source.subsys.u.pci.addr,
> &vfiofdDev) < 0)
> + return -1;
> +
> + if (!virFileExists(iommufdDir))
> + return -1;
...and even add 'bool virIOMMUFDSupported(void)' that does this
virFileExists check.
> +
> + 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
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
