* [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs
@ 2026-04-15 18:16 Matt Evans
2026-04-15 18:23 ` Leon Romanovsky
2026-04-16 8:11 ` Leon Romanovsky
0 siblings, 2 replies; 9+ messages in thread
From: Matt Evans @ 2026-04-15 18:16 UTC (permalink / raw)
To: Alex Williamson, Matt Evans, Leon Romanovsky, Jason Gunthorpe,
Kevin Tian, Vivek Kasireddy, Ankit Agrawal
Cc: kvm, linux-kernel
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@meta.com>
---
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;
+
dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges,
sizeof(*dma_ranges));
if (IS_ERR(dma_ranges))
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs 2026-04-15 18:16 [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs Matt Evans @ 2026-04-15 18:23 ` Leon Romanovsky 2026-04-16 8:11 ` Leon Romanovsky 1 sibling, 0 replies; 9+ messages in thread From: Leon Romanovsky @ 2026-04-15 18:23 UTC (permalink / raw) To: Matt Evans Cc: Alex Williamson, Jason Gunthorpe, Kevin Tian, Vivek Kasireddy, Ankit Agrawal, kvm, linux-kernel 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@meta.com> > --- > 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; I noticed this check in vfio_pci_core_mmap(). Isn't that sufficient? Thanks > + > dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges, > sizeof(*dma_ranges)); > if (IS_ERR(dma_ranges)) > -- > 2.47.3 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs 2026-04-15 18:16 [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs Matt Evans 2026-04-15 18:23 ` Leon Romanovsky @ 2026-04-16 8:11 ` Leon Romanovsky 2026-04-16 13:05 ` Matt Evans 1 sibling, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2026-04-16 8:11 UTC (permalink / raw) To: Matt Evans Cc: Alex Williamson, Jason Gunthorpe, Kevin Tian, Vivek Kasireddy, Ankit Agrawal, kvm, linux-kernel 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@meta.com> > --- > 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://sashiko.dev/#/patchset/20260415181623.1021090-1-mattev@meta.com Thanks > dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges, > sizeof(*dma_ranges)); > if (IS_ERR(dma_ranges)) > -- > 2.47.3 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs 2026-04-16 8:11 ` Leon Romanovsky @ 2026-04-16 13:05 ` Matt Evans 2026-04-16 13:14 ` Leon Romanovsky 0 siblings, 1 reply; 9+ messages in thread From: Matt Evans @ 2026-04-16 13:05 UTC (permalink / raw) To: Leon Romanovsky Cc: Alex Williamson, Jason Gunthorpe, Kevin Tian, Vivek Kasireddy, Ankit Agrawal, kvm, linux-kernel 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@meta.com> >> --- >> 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. Cheers, Matt > > Thanks > >> dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges, >> sizeof(*dma_ranges)); >> if (IS_ERR(dma_ranges)) >> -- >> 2.47.3 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs 2026-04-16 13:05 ` Matt Evans @ 2026-04-16 13:14 ` Leon Romanovsky 2026-04-16 18:03 ` Matt Evans 0 siblings, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2026-04-16 13:14 UTC (permalink / raw) To: Matt Evans Cc: Alex Williamson, Jason Gunthorpe, Kevin Tian, Vivek Kasireddy, Ankit Agrawal, kvm, linux-kernel 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@meta.com> > > > --- > > > 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. Thanks > > Cheers, > > > Matt > > > > > > Thanks > > > > > dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges, > > > sizeof(*dma_ranges)); > > > if (IS_ERR(dma_ranges)) > > > -- > > > 2.47.3 > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs 2026-04-16 13:14 ` Leon Romanovsky @ 2026-04-16 18:03 ` Matt Evans 2026-04-16 21:48 ` Alex Williamson 0 siblings, 1 reply; 9+ messages in thread From: Matt Evans @ 2026-04-16 18:03 UTC (permalink / raw) To: Leon Romanovsky Cc: Alex Williamson, Jason Gunthorpe, Kevin Tian, Vivek Kasireddy, Ankit Agrawal, kvm, linux-kernel 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@meta.com> >>>> --- >>>> 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! Thanks, Matt > > Thanks > >> >> Cheers, >> >> >> Matt >> >> >>> >>> Thanks >>> >>>> dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges, >>>> sizeof(*dma_ranges)); >>>> if (IS_ERR(dma_ranges)) >>>> -- >>>> 2.47.3 >>>> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs 2026-04-16 18:03 ` Matt Evans @ 2026-04-16 21:48 ` Alex Williamson 2026-04-17 14:25 ` Matt Evans 0 siblings, 1 reply; 9+ messages in thread From: Alex Williamson @ 2026-04-16 21:48 UTC (permalink / raw) To: Matt Evans Cc: Leon Romanovsky, Jason Gunthorpe, Kevin Tian, Vivek Kasireddy, Ankit Agrawal, kvm, linux-kernel, alex On Thu, 16 Apr 2026 19:03:40 +0100 Matt Evans <mattev@meta.com> 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@meta.com> > >>>> --- > >>>> 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, Alex ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs 2026-04-16 21:48 ` Alex Williamson @ 2026-04-17 14:25 ` Matt Evans 2026-04-17 22:31 ` Alex Williamson 0 siblings, 1 reply; 9+ messages in thread From: Matt Evans @ 2026-04-17 14:25 UTC (permalink / raw) To: Alex Williamson Cc: Leon Romanovsky, Jason Gunthorpe, Kevin Tian, Vivek Kasireddy, Ankit Agrawal, kvm, linux-kernel Hi Alex, On 16/04/2026 22:48, Alex Williamson wrote: > > On Thu, 16 Apr 2026 19:03:40 +0100 > Matt Evans <mattev@meta.com> 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@meta.com> >>>>>> --- >>>>>> 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs 2026-04-17 14:25 ` Matt Evans @ 2026-04-17 22:31 ` Alex Williamson 0 siblings, 0 replies; 9+ messages in thread From: Alex Williamson @ 2026-04-17 22:31 UTC (permalink / raw) To: Matt Evans Cc: Leon Romanovsky, Jason Gunthorpe, Kevin Tian, Vivek Kasireddy, Ankit Agrawal, kvm, linux-kernel, alex, schnelle On Fri, 17 Apr 2026 15:25:07 +0100 Matt Evans <mattev@meta.com> wrote: > On 16/04/2026 22:48, Alex Williamson wrote: > > On Thu, 16 Apr 2026 19:03:40 +0100 > > Matt Evans <mattev@meta.com> 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. > - 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? > 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.) 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. > 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, Alex ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-04-17 22:31 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-15 18:16 [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs Matt Evans 2026-04-15 18:23 ` Leon Romanovsky 2026-04-16 8:11 ` Leon Romanovsky 2026-04-16 13:05 ` Matt Evans 2026-04-16 13:14 ` Leon Romanovsky 2026-04-16 18:03 ` Matt Evans 2026-04-16 21:48 ` Alex Williamson 2026-04-17 14:25 ` Matt Evans 2026-04-17 22:31 ` Alex Williamson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox