public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [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