On Fri, Aug 21, 2026 at 12:50:23PM +0200, Martin Kletzander via Devel wrote:
> On Wed, Aug 19, 2026 at 09:53:31AM -0400, Omar Elghoul wrote:
> > On 8/19/26 3:35 AM, Martin Kletzander via Devel wrote:
> > > On Tue, Aug 18, 2026 at 12:21:03PM -0400, Omar Elghoul wrote:
> > > > Hi,
> > > >
> > > > This patchset proposes a fix for what I believe is a bug in libvirt when
> > > > using iommufd under SELinux enforcing. It is an RFC because I cannot yet
> > > > vouch for its correctness -- the goal is not to merge this code as is,
> > > > but rather start a discussion on how we should go about things.
> > > >
> > > > When using iommufd, libvirt opens the vfio cdev corresponding to the
> > > > device (e.g. /dev/vfio/devices/{n}) and the IOMMU device (/dev/iommu)
> > > > and passes the file descriptors to qemu. The cdev is a device-specific
> > > > resource, and thus a per-domain resource. The IOMMU on the other hand is
> > > > a shared resource between all domains.
> > > >
> > > > Using the SELinux driver, libvirt relabels and assigns MCS categories to
> > > > domain-specific devices to isolate from other domains. However, it also
> > > > incorrectly assigns these categories to the IOMMU device. This can make
> > > > subsequent domains unable to create DMA mappings. On my s390x host, I've
> > > > repeatedly observed this problem showing up as PCI error events that
> > > > show a DMA attempt to a non-present page.
> > > >
> > > > This problem is reproducible by running `setenforce 1` and then starting
> > > > two domains, each with a PCI passthrough device using iommufd, and then
> > > > performing actions that initiate PCI DMA transactions from within the
> > > > guests. Launching qemu manually from the command-line does not produce
> > > > these errors.
> > > >
> > > > There is also an additional cleanup problem that is not addressed in
> > > > libvirt today, nor in my patches. When libvirt relabels a device, it
> > > > doesn't restore the default label after the domain is destroyed. This is
> > > > fine for the vfio cdevs, because they will disappear anyway as they get
> > > > unbound from the vfio driver and rebind their default driver, but again
> > > > is problematic for /dev/iommu because the inode is never unlinked.
> > > >
> > > > For example, the good state before starting a domain:
> > > >
> > > > root in ~/libvirt λ matchpathcon /dev/iommu
> > > > /dev/iommu system_u:object_r:iommu_device_t:s0
> > > > root in ~/libvirt λ ls -Z /dev/iommu
> > > > system_u:object_r:iommu_device_t:s0 /dev/iommu
> > > >
> > > > After starting and ending all domains (without my patches):
> > > >
> > > > root in ~/libvirt λ ls -Z /dev/iommu
> > > > system_u:object_r:svirt_image_t:s0:c614,c980 /dev/iommu
> > > >
> > > > My patches remove the per-domain MCS categories (i.e., c614,c980) from
> > > > the IOMMU, but they do NOT clean up the inode relabel. For the domain's
> > > > lifetime, a relabel from iommu_device_t to svirt_image_t is correct. But
> > > > I'm skeptical of whether we should clean that up at all when no domains
> > > > are running, so that's another point I hope to get sorted out.
> > > >
> > > > I'd be happy to hear any feedback or thoughts on resolving this issue.
> > > > If there's more questions or anyone needs more details, I'm available
> > > > to answer those too.
> > > >
> > >
> > > It "makes sense" what libvirt does, or rather "why" it does it, but you
> > > are correct it is not correct. Sure, there are many ways this could get
> > > fixed, but what I immediately thought of was a completely different
> > > solution. I am also not sure what all the implications are and whether
> > > it is correct or not, but similarly to NFS, I feel like there could be a
> > > SELinux boolean (NFS access from virtual machines has virt_use_nfs)
> > > which would allow svirt_t to access ipmmu_device_t and with that we
> > > could stop labelling the /dev/iommu file. That would solve both of the
> > > reported issues. On the other hand, I must say I am not sure what else
> > > it would allow.
> >
> > I actually thought of this and did exactly that as an experiment to
> > verify my hypothesis prior to writing up these patches. I tested it by
> > making a SELinux module to override the default policy, letting svirt_t
> > access iommu_device_t with the relevant syscalls/ioctls/etc.
> >
> > That said, I personally didn't like that approach because I feel like
> > making a change to SELinux policy is much more likely to raise eyebrows,
>
> Which is not a bad thing, hopefully more people will chime in if that is
> needed.Hi, I've looked into this issue and IMHO we probably need to change SELinux policy and to the same as for `/dev/vfio/(vfio)?[0-9]*`. > > > and rightfully so especially when it requires a libvirt change anyway to > > drop the relabeling. Not to mention the relabeling to svirt_image_t on > > Sure, but I'm sure we can check whether the SELinux boolean exists and > behave based on that. I don't think there is a need to introduce new boolean. > > its own makes sense and is consistent with how libvirt labels other > > devices. I also feel like this can potentially leave us with weird cases > > where qemu cannot access the IOMMU at all, e.g. on a system where the > > IOMMU doesn't have the iommu_device_t label at all by default and then > > libvirt doesn't add it. > > > > What we have currently is not only multiple VMs not being able to have > a hostdev. But also other tools/programs that might require access > might be cut off. This is yet another thing that would be fixed. Agreed, and that's why I think we should update SELinux policy. Pavel > > > Thanks > > > > > > > > > Thanks in advance. > > > > > > > > Omar Elghoul (2): > > > > security: add SetSharedImageFDLabel() for shared image FDs > > > > qemu: drop per-domain MCS categories from IOMMU device > > > > > > > > src/libvirt_private.syms | 1 + > > > > src/qemu/qemu_process.c | 4 +- > > > > src/qemu/qemu_security.h | 1 + > > > > src/security/security_apparmor.c | 1 + > > > > src/security/security_dac.c | 1 + > > > > src/security/security_driver.h | 4 ++ > > > > src/security/security_manager.c | 16 ++++++++ > > > > src/security/security_manager.h | 3 ++ > > > > src/security/security_nop.c | 1 + > > > > src/security/security_selinux.c | 63 ++++++++++++++++++++++++++++++++ > > > > src/security/security_stack.c | 18 +++++++++ > > > > 11 files changed, 111 insertions(+), 2 deletions(-) > > > > > > > > -- > > > > 2.55.0 > > > > > >
signature.asc
Description: PGP signature
