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

[Matt Evans]

Hi Alex,

On 16/04/2026 22:48, Alex Williamson wrote:
> On Thu, 16 Apr 2026 19:03:40 +0100
> Matt Evans <mattev@xxxxxxxx> wrote:
>
> > Hi Leon,
> >
> > On 16/04/2026 14:14, Leon Romanovsky wrote:
> > > On Thu, Apr 16, 2026 at 02:05:30PM +0100, Matt Evans wrote:
> > > > Hi Leon,
> > > >
> > > > On 16/04/2026 09:11, Leon Romanovsky wrote:
> > > > > > On Wed, Apr 15, 2026 at 11:16:23AM -0700, Matt Evans wrote:
> > > > > > Although vfio_pci_core_feature_dma_buf() validates that both requested
> > > > > > DMABUF ranges and the PCI resources being referenced are page-aligned,
> > > > > > there may be reasons other than alignment that cause a BAR to be
> > > > > > unmappable.
> > > > > >
> > > > > > Add a check for vdev->bar_mmap_supported[index], similar to the VFIO
> > > > > > mmap path.
> > > > > >
> > > > > > Fixes: 5d74781ebc86c ("vfio/pci: Add dma-buf export support for MMIO regions")
> > > > > > Signed-off-by: Matt Evans <mattev@xxxxxxxx>
> > > > > > ---
> > > > > >     drivers/vfio/pci/vfio_pci_dmabuf.c | 3 +++
> > > > > >     1 file changed, 3 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > > > > index f87fd32e4a01..4ccaf3531e02 100644
> > > > > > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > > > > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > > > > @@ -249,6 +249,9 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
> > > > > >     	if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX)
> > > > > >     		return -ENODEV;
> > > > > > +	if (!vdev->bar_mmap_supported[get_dma_buf.region_index])
> > > > > > +		return -EINVAL;
> > > > > > +
> > > > >
> > > > > And it looks like AI has valid concern about this line too.
> > > > > https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260415181623.1021090-1-mattev@meta.com__;Iw!!Bt8RZUm9aw!5DxsN8cDUviPIZqEjG0pZ_VYYbl_RdmWucTGdTZ3ZzlVP_Ysb0n7ykr0eXwFXdpuqvZH2FK3$
> > > >
> > > > Ah, Sashiko has a point, and I think its suggestion of checking lower down
> > > > in the default .get_dmabuf_phys (vfio_pci_core_get_dmabuf_phys()) and
> > > > preserving driver overrides is decent.  Will revisit.
> > > >
> > > > To your other question:
> > > > > I noticed this check in vfio_pci_core_mmap(). Isn't that sufficient?
> > > >
> > > > The scenario in mind is doing a DMABUF-export for BARs that you haven't
> > > > necessarily noticed can't be mmap()ed, and both paths should be checking.
> > >
> > > I added the validation checks that matter on the kernel side, but mmap is
> > > primarily important for callers. What I am missing is an explanation of
> > > why the kernel should impose this restriction on itself.
> >
> > 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.)

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).

- 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).

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.)

(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.)

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. :)

Matt