Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs

[Matt Evans]

Hi Alex,

On 17/04/2026 23:31, Alex Williamson wrote:
> On Fri, 17 Apr 2026 15:25:07 +0100
> Matt Evans <mattev@xxxxxxxx> wrote:
> > On 16/04/2026 22:48, Alex Williamson wrote:
> > > On Thu, 16 Apr 2026 19:03:40 +0100
> > > Matt Evans <mattev@xxxxxxxx> wrote:
> > > > On 16/04/2026 14:14, Leon Romanovsky wrote:
> > > > > On Thu, Apr 16, 2026 at 02:05:30PM +0100, Matt Evans wrote:
> > > >
> > > > I don't understand your question, really sorry!  Can you rephrase it
> > > > please?  I want to make sure I answer it fully.
> > > >
> > > > Although mmap() fails for BARs that are unmappable (for whatever
> > > > reason), a DMABUF export for the same ones could in some slim cases
> > > > succeed -- because the checks aren't identical.  If export succeeds, it
> > > > could potentially allow P2P (or CPU via a future DMABUF mmap()) access
> > > > to something possibly unmappable, no?
> > > >
> > > > For the checks that vfio_pci_probe_mmaps() does (leading to
> > > > bar_mmap_supported[] = false), most have corresponding-but-different
> > > > checks reachable from DMABUF export:
> > > >
> > > > If a BAR is:		Then DMABUF export...:
> > > >
> > > >     size < pagesize	vfio_pci_core_fill_phys_vec() catches it
> > > >     Not IORESOURCE_MEM	pcim_p2pdma_provider() rejects it
> > > >     non_mappable_bars	... nothing?  Export allowed
> > > >
> > > > As a quick test, if I hack in non_mappable_bars=1 for my function, it
> > > > appears exporting a DMABUF from it works.
> > > >
> > > > We could add another check for non_mappable_bars, but my thinking was
> > > > that we don't want to keep adding to an independent set of DMABUF
> > > > checks, especially if a future quirk/etc. could create another scenario
> > > > where BARs aren't mappable.  I.e. we should reject DMABUF export in
> > > > exactly the same scenarios as mmap() would be rejected, symmetrically,
> > > > by testing bar_mmap_supported[].
> > > >
> > > > Hope that goes some way to answering the Q, hopefully I haven't missed
> > > > something!
> > >
> > > That's the concern as I see it as well, it's a choice whether to
> > > attempt to support sub-PAGE_SIZE mappings, but if a device is reporting
> > > non_mappable_bars are we're exporting those BARs through dma-buf for
> > > mmap, that's a problem.  Should pcim_p2pdma_provider() test this flag
> > rather than vfio_pci_dmabuf though?  Thanks,
> >
> > Do you mean "test this flag" to say 'non_mappable_bars' rather than the
> > 'vdev->bar_mmap_supported[]' flag?  (I think the former, as the latter
> > isn't as easily available down there, sorry if that's not what you
> > intended.)
>
> Yes
>
> > Testing non_mappable_bars in pcim_p2pdma_provider() doesn't feel quite
> > right:
> >
> > - IIUC non_mappable_bars is about preventing mapping to userspace, not
> > direct access/P2P/etc.  Splitting hairs a bit, but P2P to such a device
> > seems valid, so I think this check better stays within vfio-pci
> > (specifically limiting only userspace access).
>
> Indeed the flag does specify userspace mapping in the comment, but I
> think it's more than that.  The only device that currently uses it is a
> device unique to s390x, where AIUI, there is no P2P DMA anyway.
>
> Also, for "legacy" mapping of device BARs through the vfio type1
> backend, the mechanics of the mapping require the BAR can be mmap'd
> such that the user virtual address can then be used for the DMA
> mapping.  So as far as vfio has traditionally been concerned, there's
> an inherent dependency.
>
> I could definitely see that p2p folks could balk and we need a separate
> flag... or since there's exactly one device that reports this flag,
> maybe we can tweak the description.

Thanks for explaining; with that context, ...

> > - But non_mappable_bars is just one kind of quirk, and couldn't there
> > possibly be more?  Good to avoid duplicating quirk checks in mmap() &
> > export places (so any future maintenance updates just one place and no
> > disparities arise).
>
> I'm not sure what this is getting at, I think non_mappable_bars is
> meant to be the flag that quirks might set if for any reason we can't
> directly map the BAR.  I think "userspace" mapping is a bit of a
> red-herring, it's at least not mappable by the CPU, but I don't think
> the flag is actually meant to define a policy only for userspace.  If
> it's not mappable by the CPU... is that also enough of a restriction to
> exclude it from P2P mappings?

...and this, it makes sense then to just check non_mappable_bars, if 
it's not about userspace and has the semantic of "don't access this 
directly".

It'd be nice if it evolved to a per-BAR flag rather than per-function, 
BUT I acknowledge I was largely worrying about the shape of hypothetical 
future quirks and that isn't necessary just now.

> > All this to say:   The existng logic in vfio_pci_probe_mmaps() is the
> > right place to decide something is suitable for userspace/direct
> > mapping (mappable, non-zero sized, not IORESOURCE_IO), IMHO we just
> > need to be checking it consistently.  (VFIO export is less lenient in
> > terms of >= pagesize and imposes additional checks, and that's good
> > as long as the checks are an overlay rather than parallel
> > duplication.)
>
> I'd actually take the opposite stance and say that vfio is not the only
> consumer of pcim_p2pdma_provider() and if a device has a flag that it
> shouldn't be mapped, the p2p subsystem should also honor that, not just
> vfio.
>
> > (Maybe you're also flagging that there could be other checks saying "Is
> > P2P supported for this BAR?" and they could be done in a generic
> > drivers/pci place?  I think so;  VFIO export criteria be the
> > intersection of VFIO- and provider-based checks.)

(Typo, inadvertent talk like a pirate.)

> Yes (I think).  If we determine that non_mappable_bars means both CPU
> and device mappings (which is compatible with its current use case),
> then fixing pcim_p2pdma_provider() to honor the flag fixes all
> consumers.

OK, makes sense.  Seems OK to couple CPU mappings and P2P; I can imagine 
quirks (particularly for RCiEPs) where one works but the other doesn't 
but, until we've a concrete example, affecting both is safest.

For now, I'll do as you first suggested ( :) ), and if future 
more-elaborate quirks arise we can revisit.

> > I was thinking something like...
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > index 00cedfe3a57d..9bb8bd153e12 100644
> > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > @@ -359,6 +359,9 @@ int vfio_pci_core_get_dmabuf_phys(struct
> > vfio_pci_core_device *vdev,
> >           if (!*provider)
> >                   return -EINVAL;
> >
> > +       if (!vdev->bar_mmap_supported[region_index])
> > +               return -EINVAL;
> > +
> >           return vfio_pci_core_fill_phys_vec(
> >                   phys_vec, dma_ranges, nr_ranges,
> >                   pci_resource_start(pdev, region_index),
> >
> > (I.e. leverage logic in vfio_pci_probe_mmaps(), catch bad DMABUF
> > mappings this way, allows sub-drivers to override .get_dmabuf_phys.)
> >
> > I'll repost that as v2 if this doesn't seem an outrageous start. :)
>
> That's fixing the leaf driver rather than the subsystem, where
> pci/p2pdma really ought to honor its own flag indicating the BAR is not
> mappable.  The precedent is already there in rejecting IO BARs.  Thanks,

All good; I appreciate the discussion, thanks, and will redo with 
checking non_mappable_bars in p2pdma.


Matt