* [PATCH v3 0/3] vfio/pci: Request resources and map BARs at enable time
@ 2026-04-30 10:03 Matt Evans
2026-04-30 10:03 ` [PATCH v3 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable() Matt Evans
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Matt Evans @ 2026-04-30 10:03 UTC (permalink / raw)
To: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal,
Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum,
Yishai Hadas
Cc: Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
Hi,
These patches fix a potential race for concurrent calls to
vfio_pci_core_setup_barmap(), and a DMABUF missing check for resource
before the export. Discussion on a previous series (different,
replaced by this one) is here:
https://lore.kernel.org/kvm/20260415181423.1008458-1-mattev@meta.com
Responses in that thread indicated there wasn't a strong historical
reason to require the mapping to be performed on-demand at BAR
reference time. It's much simpler to move this earlier, to
vfio_pci_core_enable(), and that then avoids having to deal with
concurrent requests later.
The first patch requests PCI resources and pci_iomap() of the BARs
from vfio_pci_core_enable(), moving this out of
vfio_pci_core_setup_barmap().
Some callers rely on vfio_pci_core_setup_barmap() for its ioremap()
effect, and other callers use it for its resource-acquiring effect.
The function turns into a cheap error check that both these actions
have occurred and keeps the same error behaviour.
The second patch refactors that function plus the various
vdev->barmap[] accesses into vfio_pci_core_get_iomap() which returns
either a pointer to the mapping or an ERR_PTR() describing why it
doesn't exist. This is used by callers that need the mapping, but
also by other callers to check that the resource/mapping step was
successful.
NOTE: This removes the EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap).
It does not re-add an export for vfio_pci_core_get_iomap() yet. (I
wanted to check the preference/policy here.)
The third patch adds the resource check to VFIO DMABUF export, which
was previously able to export an unrequested resource. Although patch
1 at first appears to fix this by requesting resources at enable time,
code using the BAR still needs to check the resource really was
acquired.
=== Changes ===
v3:
- Remove the separate tracking of the BAR mapping versus the
acquiring its resource. Errors from failing iomap vs resource
reservation are ERR_PTR()-elcoded into barmap[bar].
- Remove the separate test helper, and add vfio_pci_core_get_iomap().
This gets the iomap base or is used check for error/failure to
acquire the resource. Added comments at call sites explaining
whether they want to just ensure the resource is reserved versus
actually use the mapping.
v2:
https://lore.kernel.org/kvm/20260423182517.2286030-1-mattev@meta.com/
- Don't fail if resources can't be requested or iomapped, even for
valid BARs, as this would change the userspace-observable error
behaviour. Specifically, if there was an issue with one particular
BAR which happened to never be used, then userspace would never
encounter an error for it. Track iomap and resource-acquisition
status per BAR.
- Break out the checks for resource success from those for iomap
success, in the form of the two new helpers.
- Third patch to add the check to VFIO DMABUF export, because
init-time requests can now fail.
v1:
https://lore.kernel.org/kvm/20260421174143.3883579-1-mattev@meta.com/
Matt Evans (3):
vfio/pci: Set up bar resources and maps in vfio_pci_core_enable()
vfio/pci: Replace vfio_pci_core_setup_barmap() with
vfio_pci_core_get_iomap()
vfio/pci: Check BAR resources before exporting a DMABUF
drivers/vfio/pci/nvgrace-gpu/main.c | 17 ++++++-----
drivers/vfio/pci/vfio_pci_core.c | 44 +++++++++++++++++++++++++----
drivers/vfio/pci/vfio_pci_dmabuf.c | 6 ++--
drivers/vfio/pci/vfio_pci_rdwr.c | 42 ++++++---------------------
drivers/vfio/pci/virtio/legacy_io.c | 13 ++++-----
include/linux/vfio_pci_core.h | 19 ++++++++++++-
6 files changed, 84 insertions(+), 57 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v3 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable() 2026-04-30 10:03 [PATCH v3 0/3] vfio/pci: Request resources and map BARs at enable time Matt Evans @ 2026-04-30 10:03 ` Matt Evans 2026-04-30 20:13 ` Alex Williamson 2026-04-30 10:03 ` [PATCH v3 2/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with vfio_pci_core_get_iomap() Matt Evans 2026-04-30 10:03 ` [PATCH v3 3/3] vfio/pci: Check BAR resources before exporting a DMABUF Matt Evans 2 siblings, 1 reply; 8+ messages in thread From: Matt Evans @ 2026-04-30 10:03 UTC (permalink / raw) To: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas Cc: Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy, Zhi Wang, kvm, linux-kernel, virtualization Previously BAR resource requests and the corresponding pci_iomap() were performed on-demand and without synchronisation, which was racy. Rather than add synchronisation, it's simplest to address this by doing both activities from vfio_pci_core_enable(). The resource allocation and/or pci_iomap() can still fail; their status is tracked and existing calls to vfio_pci_core_setup_barmap() will fail in a similar way to before. This keeps the point of failure as observed by userspace the same, i.e. failures to request/map unused BARs are benign. Fixes: 7f5764e179c6 ("vfio: use vfio_pci_core_setup_barmap to map bar in mmap") Fixes: 0d77ed3589ac0 ("vfio/pci: Pull BAR mapping setup from read-write path") Signed-off-by: Matt Evans <mattev@meta.com> --- drivers/vfio/pci/vfio_pci_core.c | 33 ++++++++++++++++++++++++++++++++ drivers/vfio/pci/vfio_pci_rdwr.c | 29 ++++++++++++---------------- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 3f8d093aacf8..eab4f2626b39 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -482,6 +482,38 @@ static int vfio_pci_core_runtime_resume(struct device *dev) } #endif /* CONFIG_PM */ +static void vfio_pci_core_map_bars(struct vfio_pci_core_device *vdev) +{ + struct pci_dev *pdev = vdev->pdev; + int i; + + /* + * Eager-request BAR resources, and iomap. Soft failures are + * allowed, and consumers must check the barmap before use in + * order to give compatible user-visible behaviour with the + * previous on-demand allocation method. + */ + for (i = 0; i < PCI_STD_NUM_BARS; i++) { + int bar = i + PCI_STD_RESOURCES; + void __iomem *io = ERR_PTR(-ENODEV); + + if (pci_resource_len(pdev, i) > 0) { + if (pci_request_selected_regions(pdev, 1 << bar, "vfio")) { + pci_warn(vdev->pdev, "Failed to reserve region %d\n", bar); + io = ERR_PTR(-EBUSY); + } else { + io = pci_iomap(pdev, bar, 0); + if (!io) { + pci_warn(vdev->pdev, "Failed to iomap region %d\n", + bar); + io = ERR_PTR(-ENOMEM); + } + } + } + vdev->barmap[bar] = io; + } +} + /* * The pci-driver core runtime PM routines always save the device state * before going into suspended state. If the device is going into low power @@ -568,6 +600,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev) if (!vfio_vga_disabled() && vfio_pci_is_vga(pdev)) vdev->has_vga = true; + vfio_pci_core_map_bars(vdev); return 0; diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c index 4251ee03e146..f66ad3d96481 100644 --- a/drivers/vfio/pci/vfio_pci_rdwr.c +++ b/drivers/vfio/pci/vfio_pci_rdwr.c @@ -200,25 +200,20 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw); int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar) { - struct pci_dev *pdev = vdev->pdev; - int ret; - void __iomem *io; - - if (vdev->barmap[bar]) - return 0; - - ret = pci_request_selected_regions(pdev, 1 << bar, "vfio"); - if (ret) - return ret; - - io = pci_iomap(pdev, bar, 0); - if (!io) { - pci_release_selected_regions(pdev, 1 << bar); - return -ENOMEM; - } + /* + * The barmap is set up in vfio_pci_core_enable(). Callers + * use this function to check that the BAR resources are + * requested or that the pci_iomap() was done. + */ + if (bar < 0 || bar >= PCI_STD_NUM_BARS) + return -EINVAL; - vdev->barmap[bar] = io; + /* Did vfio_pci_core_map_bars() set it up yet? */ + if (!vdev->barmap[bar]) + return -ENODEV; + if (IS_ERR(vdev->barmap[bar])) + return PTR_ERR(vdev->barmap[bar]); return 0; } EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap); -- 2.47.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable() 2026-04-30 10:03 ` [PATCH v3 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable() Matt Evans @ 2026-04-30 20:13 ` Alex Williamson 2026-05-05 16:40 ` Matt Evans 0 siblings, 1 reply; 8+ messages in thread From: Alex Williamson @ 2026-04-30 20:13 UTC (permalink / raw) To: Matt Evans Cc: Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas, Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy, Zhi Wang, kvm, linux-kernel, virtualization, alex On Thu, 30 Apr 2026 03:03:20 -0700 Matt Evans <mattev@meta.com> wrote: > Previously BAR resource requests and the corresponding pci_iomap() > were performed on-demand and without synchronisation, which was racy. > Rather than add synchronisation, it's simplest to address this by > doing both activities from vfio_pci_core_enable(). > > The resource allocation and/or pci_iomap() can still fail; their > status is tracked and existing calls to vfio_pci_core_setup_barmap() > will fail in a similar way to before. This keeps the point of failure > as observed by userspace the same, i.e. failures to request/map unused > BARs are benign. > > Fixes: 7f5764e179c6 ("vfio: use vfio_pci_core_setup_barmap to map bar in mmap") > Fixes: 0d77ed3589ac0 ("vfio/pci: Pull BAR mapping setup from read-write path") Neither of these introduced races, they only moved what they were already doing into a function or made use of that shared function for what they were already doing. I'm inclined to believe the raciness existed from the introduction, 89e1f7d4c66d. > Signed-off-by: Matt Evans <mattev@meta.com> > --- > drivers/vfio/pci/vfio_pci_core.c | 33 ++++++++++++++++++++++++++++++++ > drivers/vfio/pci/vfio_pci_rdwr.c | 29 ++++++++++++---------------- > 2 files changed, 45 insertions(+), 17 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c > index 3f8d093aacf8..eab4f2626b39 100644 > --- a/drivers/vfio/pci/vfio_pci_core.c > +++ b/drivers/vfio/pci/vfio_pci_core.c > @@ -482,6 +482,38 @@ static int vfio_pci_core_runtime_resume(struct device *dev) > } > #endif /* CONFIG_PM */ > > +static void vfio_pci_core_map_bars(struct vfio_pci_core_device *vdev) > +{ > + struct pci_dev *pdev = vdev->pdev; > + int i; > + > + /* > + * Eager-request BAR resources, and iomap. Soft failures are > + * allowed, and consumers must check the barmap before use in > + * order to give compatible user-visible behaviour with the > + * previous on-demand allocation method. > + */ > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > + int bar = i + PCI_STD_RESOURCES; > + void __iomem *io = ERR_PTR(-ENODEV); It would collapse the nesting depth to just do: vdev->barmap[bar] = ERR_PTR(-ENODEV); if (!pci_resource_len(pdev, i)) continue; if (pci_request_selected_regions(pdev, 1 << bar, "vfio")) { pci_dbg(vdev->pdev, "Failed to reserve region %d\n", bar); vdev->barmap[bar] = ERR_PTR(-EBUSY); continue; } vdev->barmap[bar] = pci_iomap(pdev, bar, 0); if (!vdev->barmap[bar]) { pci_dbg(vdev->pdev, "Failed to iomap region %d\n", bar); vdev->barmap[bar] = ERR_PTR(-ENOMEM); } It's debatable what level to use for the errors, but we were previously silent on this, so going all the way to pci_warn() seems unnecessary. > + > + if (pci_resource_len(pdev, i) > 0) { > + if (pci_request_selected_regions(pdev, 1 << bar, "vfio")) { > + pci_warn(vdev->pdev, "Failed to reserve region %d\n", bar); > + io = ERR_PTR(-EBUSY); > + } else { > + io = pci_iomap(pdev, bar, 0); > + if (!io) { > + pci_warn(vdev->pdev, "Failed to iomap region %d\n", > + bar); > + io = ERR_PTR(-ENOMEM); > + } > + } > + } > + vdev->barmap[bar] = io; > + } > +} > + > /* > * The pci-driver core runtime PM routines always save the device state > * before going into suspended state. If the device is going into low power > @@ -568,6 +600,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev) > if (!vfio_vga_disabled() && vfio_pci_is_vga(pdev)) > vdev->has_vga = true; > > + vfio_pci_core_map_bars(vdev); > > return 0; You're missing the barmap test in vfio_pci_core_disable() now, it's still testing for NULL, which is (almost?) never true. It needs to convert to IS_ERR_OR_NULL(). > > diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c > index 4251ee03e146..f66ad3d96481 100644 > --- a/drivers/vfio/pci/vfio_pci_rdwr.c > +++ b/drivers/vfio/pci/vfio_pci_rdwr.c > @@ -200,25 +200,20 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw); > > int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar) > { > - struct pci_dev *pdev = vdev->pdev; > - int ret; > - void __iomem *io; > - > - if (vdev->barmap[bar]) > - return 0; > - > - ret = pci_request_selected_regions(pdev, 1 << bar, "vfio"); > - if (ret) > - return ret; > - > - io = pci_iomap(pdev, bar, 0); > - if (!io) { > - pci_release_selected_regions(pdev, 1 << bar); > - return -ENOMEM; > - } > + /* > + * The barmap is set up in vfio_pci_core_enable(). Callers > + * use this function to check that the BAR resources are > + * requested or that the pci_iomap() was done. > + */ Looks like a function level comment to be placed above the function definition. TBH, the comment in the previous function could also be pulled up as a function level comment. > + if (bar < 0 || bar >= PCI_STD_NUM_BARS) Maybe `if ((unsigned)bar >= PCI_STD_NUM_BARS)` but really author preference here. > + return -EINVAL; > > - vdev->barmap[bar] = io; > + /* Did vfio_pci_core_map_bars() set it up yet? */ > + if (!vdev->barmap[bar]) > + return -ENODEV; What hits this? Should it be a WARN_ON_ONCE? It would need to be a use case that accesses barmap outside of the window between enable and disable, where I think we're defining the contract that it's only valid between those events. Both this and the range check could move to the iomap implemenation to keep the Fixes: patch reasonably small since afaik they're not triggered. The BAR range test could be WARN_ON_ONCE as well, only driver bugs should hit it. Thanks, Alex > > + if (IS_ERR(vdev->barmap[bar])) > + return PTR_ERR(vdev->barmap[bar]); > return 0; > } > EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable() 2026-04-30 20:13 ` Alex Williamson @ 2026-05-05 16:40 ` Matt Evans 0 siblings, 0 replies; 8+ messages in thread From: Matt Evans @ 2026-05-05 16:40 UTC (permalink / raw) To: Alex Williamson Cc: Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas, Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy, Zhi Wang, kvm, linux-kernel, virtualization Hi Alex, On 30/04/2026 21:13, Alex Williamson wrote: > > On Thu, 30 Apr 2026 03:03:20 -0700 > Matt Evans <mattev@meta.com> wrote: > >> Previously BAR resource requests and the corresponding pci_iomap() >> were performed on-demand and without synchronisation, which was racy. >> Rather than add synchronisation, it's simplest to address this by >> doing both activities from vfio_pci_core_enable(). >> >> The resource allocation and/or pci_iomap() can still fail; their >> status is tracked and existing calls to vfio_pci_core_setup_barmap() >> will fail in a similar way to before. This keeps the point of failure >> as observed by userspace the same, i.e. failures to request/map unused >> BARs are benign. >> >> Fixes: 7f5764e179c6 ("vfio: use vfio_pci_core_setup_barmap to map bar in mmap") >> Fixes: 0d77ed3589ac0 ("vfio/pci: Pull BAR mapping setup from read-write path") > > Neither of these introduced races, they only moved what they were > already doing into a function or made use of that shared function for > what they were already doing. I'm inclined to believe the raciness > existed from the introduction, 89e1f7d4c66d. > >> Signed-off-by: Matt Evans <mattev@meta.com> >> --- >> drivers/vfio/pci/vfio_pci_core.c | 33 ++++++++++++++++++++++++++++++++ >> drivers/vfio/pci/vfio_pci_rdwr.c | 29 ++++++++++++---------------- >> 2 files changed, 45 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c >> index 3f8d093aacf8..eab4f2626b39 100644 >> --- a/drivers/vfio/pci/vfio_pci_core.c >> +++ b/drivers/vfio/pci/vfio_pci_core.c >> @@ -482,6 +482,38 @@ static int vfio_pci_core_runtime_resume(struct device *dev) >> } >> #endif /* CONFIG_PM */ >> >> +static void vfio_pci_core_map_bars(struct vfio_pci_core_device *vdev) >> +{ >> + struct pci_dev *pdev = vdev->pdev; >> + int i; >> + >> + /* >> + * Eager-request BAR resources, and iomap. Soft failures are >> + * allowed, and consumers must check the barmap before use in >> + * order to give compatible user-visible behaviour with the >> + * previous on-demand allocation method. >> + */ >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> + int bar = i + PCI_STD_RESOURCES; >> + void __iomem *io = ERR_PTR(-ENODEV); > > It would collapse the nesting depth to just do: > > vdev->barmap[bar] = ERR_PTR(-ENODEV); > > if (!pci_resource_len(pdev, i)) > continue; > > if (pci_request_selected_regions(pdev, 1 << bar, "vfio")) { > pci_dbg(vdev->pdev, "Failed to reserve region %d\n", bar); > vdev->barmap[bar] = ERR_PTR(-EBUSY); > continue; > } > > vdev->barmap[bar] = pci_iomap(pdev, bar, 0); > if (!vdev->barmap[bar]) { > pci_dbg(vdev->pdev, "Failed to iomap region %d\n", bar); > vdev->barmap[bar] = ERR_PTR(-ENOMEM); > } > > It's debatable what level to use for the errors, but we were previously > silent on this, so going all the way to pci_warn() seems unnecessary. Hm, okay, returned it to a nesting-less format and replaced pci_warn()s with pci_dbg(). >> + >> + if (pci_resource_len(pdev, i) > 0) { >> + if (pci_request_selected_regions(pdev, 1 << bar, "vfio")) { >> + pci_warn(vdev->pdev, "Failed to reserve region %d\n", bar); >> + io = ERR_PTR(-EBUSY); >> + } else { >> + io = pci_iomap(pdev, bar, 0); >> + if (!io) { >> + pci_warn(vdev->pdev, "Failed to iomap region %d\n", >> + bar); >> + io = ERR_PTR(-ENOMEM); >> + } >> + } >> + } >> + vdev->barmap[bar] = io; >> + } >> +} >> + >> /* >> * The pci-driver core runtime PM routines always save the device state >> * before going into suspended state. If the device is going into low power >> @@ -568,6 +600,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev) >> if (!vfio_vga_disabled() && vfio_pci_is_vga(pdev)) >> vdev->has_vga = true; >> >> + vfio_pci_core_map_bars(vdev); >> >> return 0; > > You're missing the barmap test in vfio_pci_core_disable() now, it's > still testing for NULL, which is (almost?) never true. It needs to > convert to IS_ERR_OR_NULL(). Arrrrgh, yes it does, thank you. (For the second time, the first being the !IS_ERR() typo you caught in patch #3 :( Thanks there also; it slipped by my usual testing routine.) >> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c >> index 4251ee03e146..f66ad3d96481 100644 >> --- a/drivers/vfio/pci/vfio_pci_rdwr.c >> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c >> @@ -200,25 +200,20 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw); >> >> int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar) >> { >> - struct pci_dev *pdev = vdev->pdev; >> - int ret; >> - void __iomem *io; >> - >> - if (vdev->barmap[bar]) >> - return 0; >> - >> - ret = pci_request_selected_regions(pdev, 1 << bar, "vfio"); >> - if (ret) >> - return ret; >> - >> - io = pci_iomap(pdev, bar, 0); >> - if (!io) { >> - pci_release_selected_regions(pdev, 1 << bar); >> - return -ENOMEM; >> - } >> + /* >> + * The barmap is set up in vfio_pci_core_enable(). Callers >> + * use this function to check that the BAR resources are >> + * requested or that the pci_iomap() was done. >> + */ > > Looks like a function level comment to be placed above the function > definition. TBH, the comment in the previous function could also be > pulled up as a function level comment. > >> + if (bar < 0 || bar >= PCI_STD_NUM_BARS) > > Maybe `if ((unsigned)bar >= PCI_STD_NUM_BARS)` but really author > preference here. > >> + return -EINVAL; >> >> - vdev->barmap[bar] = io; >> + /* Did vfio_pci_core_map_bars() set it up yet? */ >> + if (!vdev->barmap[bar]) >> + return -ENODEV; > > What hits this? Should it be a WARN_ON_ONCE? It would need to be a use > case that accesses barmap outside of the window between enable and > disable, where I think we're defining the contract that it's only valid > between those events. Both this and the range check could move to the > iomap implemenation to keep the Fixes: patch reasonably small since > afaik they're not triggered. The BAR range test could be WARN_ON_ONCE > as well, only driver bugs should hit it. Thanks, I've reduced the fix patch #1 to just an IS_ERR test (without the null or range checks as you suggest). And indeed WARN_ON_ONCE() is a good idea as only tremendous mishaps would lead to these conditions triggering (worth testing though). Also ack on your suggestion on patch #2 to make the call to nvgrace_gpu_wait_device_ready() more minimalist, and to order the 2x fixes up front. Posting v4 shortly, cheers! Thanks, Matt ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with vfio_pci_core_get_iomap() 2026-04-30 10:03 [PATCH v3 0/3] vfio/pci: Request resources and map BARs at enable time Matt Evans 2026-04-30 10:03 ` [PATCH v3 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable() Matt Evans @ 2026-04-30 10:03 ` Matt Evans 2026-04-30 20:13 ` Alex Williamson 2026-04-30 10:03 ` [PATCH v3 3/3] vfio/pci: Check BAR resources before exporting a DMABUF Matt Evans 2 siblings, 1 reply; 8+ messages in thread From: Matt Evans @ 2026-04-30 10:03 UTC (permalink / raw) To: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas Cc: Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy, Zhi Wang, kvm, linux-kernel, virtualization Since "vfio/pci: Set up barmap in vfio_pci_core_enable()", the resource request and iomap for the BARs was performed early, and vfio_pci_core_setup_barmap() just checks those actions succeeded. Move this logic to a new helper that checks success and returns the iomap address, replacing the various bare vdev->barmap[] lookups. This maintains the error behaviour of the previous on-demand vfio_pci_core_setup_barmap() scheme. Signed-off-by: Matt Evans <mattev@meta.com> --- drivers/vfio/pci/nvgrace-gpu/main.c | 17 +++++++------ drivers/vfio/pci/vfio_pci_core.c | 11 ++++----- drivers/vfio/pci/vfio_pci_rdwr.c | 37 +++++++---------------------- drivers/vfio/pci/virtio/legacy_io.c | 13 +++++----- include/linux/vfio_pci_core.h | 19 ++++++++++++++- 5 files changed, 47 insertions(+), 50 deletions(-) diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c index fa056b69f899..2f5ec60c15d9 100644 --- a/drivers/vfio/pci/nvgrace-gpu/main.c +++ b/drivers/vfio/pci/nvgrace-gpu/main.c @@ -184,13 +184,11 @@ static int nvgrace_gpu_open_device(struct vfio_device *core_vdev) /* * GPU readiness is checked by reading the BAR0 registers. - * - * ioremap BAR0 to ensure that the BAR0 mapping is present before - * register reads on first fault before establishing any GPU - * memory mapping. + * The BAR map was just set up by vfio_pci_core_enable() and, + * although the readiness check checks validity of the BAR0 + * map, assert early that the map was successful: */ - ret = vfio_pci_core_setup_barmap(vdev, 0); - if (ret) + if (IS_ERR(vfio_pci_core_get_iomap(vdev, 0))) goto error_exit; if (nvdev->resmem.memlength) { @@ -265,6 +263,7 @@ static int nvgrace_gpu_check_device_ready(struct nvgrace_gpu_pci_core_device *nvdev) { struct vfio_pci_core_device *vdev = &nvdev->core_device; + void __iomem *io; int ret; lockdep_assert_held_read(&vdev->memory_lock); @@ -275,7 +274,11 @@ nvgrace_gpu_check_device_ready(struct nvgrace_gpu_pci_core_device *nvdev) if (!__vfio_pci_memory_enabled(vdev)) return -EIO; - ret = nvgrace_gpu_wait_device_ready(vdev->barmap[0]); + io = vfio_pci_core_get_iomap(vdev, 0); + if (IS_ERR(io)) + return PTR_ERR(io); + + ret = nvgrace_gpu_wait_device_ready(io); if (ret) return ret; diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index eab4f2626b39..feaf894ac118 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1760,7 +1760,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma struct pci_dev *pdev = vdev->pdev; unsigned int index; u64 phys_len, req_len, pgoff, req_start; - int ret; + void __iomem *bar_io; index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); @@ -1794,12 +1794,11 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma return -EINVAL; /* - * Even though we don't make use of the barmap for the mmap, - * we need to request the region and the barmap tracks that. + * Ensure the BAR resource region is reserved for use. */ - ret = vfio_pci_core_setup_barmap(vdev, index); - if (ret) - return ret; + bar_io = vfio_pci_core_get_iomap(vdev, index); + if (IS_ERR(bar_io)) + return PTR_ERR(bar_io); vma->vm_private_data = vdev; vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c index f66ad3d96481..7f14dd46de17 100644 --- a/drivers/vfio/pci/vfio_pci_rdwr.c +++ b/drivers/vfio/pci/vfio_pci_rdwr.c @@ -198,26 +198,6 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, } EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw); -int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar) -{ - /* - * The barmap is set up in vfio_pci_core_enable(). Callers - * use this function to check that the BAR resources are - * requested or that the pci_iomap() was done. - */ - if (bar < 0 || bar >= PCI_STD_NUM_BARS) - return -EINVAL; - - /* Did vfio_pci_core_map_bars() set it up yet? */ - if (!vdev->barmap[bar]) - return -ENODEV; - - if (IS_ERR(vdev->barmap[bar])) - return PTR_ERR(vdev->barmap[bar]); - return 0; -} -EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap); - ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf, size_t count, loff_t *ppos, bool iswrite) { @@ -269,13 +249,11 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf, */ max_width = VFIO_PCI_IO_WIDTH_4; } else { - int ret = vfio_pci_core_setup_barmap(vdev, bar); - if (ret) { - done = ret; + io = vfio_pci_core_get_iomap(vdev, bar); + if (IS_ERR(io)) { + done = PTR_ERR(io); goto out; } - - io = vdev->barmap[bar]; } if (bar == vdev->msix_bar) { @@ -430,6 +408,7 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset, loff_t pos = offset & VFIO_PCI_OFFSET_MASK; int ret, bar = VFIO_PCI_OFFSET_TO_INDEX(offset); struct vfio_pci_ioeventfd *ioeventfd; + void __iomem *io; /* Only support ioeventfds into BARs */ if (bar > VFIO_PCI_BAR5_REGION_INDEX) @@ -447,9 +426,9 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset, if (count == 8) return -EINVAL; - ret = vfio_pci_core_setup_barmap(vdev, bar); - if (ret) - return ret; + io = vfio_pci_core_get_iomap(vdev, bar); + if (IS_ERR(io)) + return PTR_ERR(io); mutex_lock(&vdev->ioeventfds_lock); @@ -486,7 +465,7 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset, } ioeventfd->vdev = vdev; - ioeventfd->addr = vdev->barmap[bar] + pos; + ioeventfd->addr = io + pos; ioeventfd->data = data; ioeventfd->pos = pos; ioeventfd->bar = bar; diff --git a/drivers/vfio/pci/virtio/legacy_io.c b/drivers/vfio/pci/virtio/legacy_io.c index 1ed349a55629..c868b2177310 100644 --- a/drivers/vfio/pci/virtio/legacy_io.c +++ b/drivers/vfio/pci/virtio/legacy_io.c @@ -299,19 +299,18 @@ int virtiovf_pci_ioctl_get_region_info(struct vfio_device *core_vdev, static int virtiovf_set_notify_addr(struct virtiovf_pci_core_device *virtvdev) { struct vfio_pci_core_device *core_device = &virtvdev->core_device; - int ret; + void __iomem *io; /* * Setup the BAR where the 'notify' exists to be used by vfio as well * This will let us mmap it only once and use it when needed. */ - ret = vfio_pci_core_setup_barmap(core_device, - virtvdev->notify_bar); - if (ret) - return ret; + io = vfio_pci_core_get_iomap(core_device, + virtvdev->notify_bar); + if (IS_ERR(io)) + return PTR_ERR(io); - virtvdev->notify_addr = core_device->barmap[virtvdev->notify_bar] + - virtvdev->notify_offset; + virtvdev->notify_addr = io + virtvdev->notify_offset; return 0; } diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index 2ebba746c18f..5598071c5ea3 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -188,7 +188,6 @@ int vfio_pci_core_match_token_uuid(struct vfio_device *core_vdev, int vfio_pci_core_enable(struct vfio_pci_core_device *vdev); void vfio_pci_core_disable(struct vfio_pci_core_device *vdev); void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev); -int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar); pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev, pci_channel_state_t state); ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, @@ -234,6 +233,24 @@ static inline bool is_aligned_for_order(struct vm_area_struct *vma, !IS_ALIGNED(pfn, 1 << order))); } +/* + * Returns a BAR's iomap base, or an ERR_PTR() if, for example, the + * BAR isn't valid, its resource wasn't acquired, or its iomap + * failed. + */ +static inline void __iomem __must_check * +vfio_pci_core_get_iomap(struct vfio_pci_core_device *vdev, int bar) +{ + if (bar < 0 || bar >= PCI_STD_NUM_BARS) + return ERR_PTR(-EINVAL); + + /* Did vfio_pci_core_map_bars() set it up yet? */ + if (!vdev->barmap[bar]) + return ERR_PTR(-ENODEV); + + return vdev->barmap[bar]; +} + int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, struct phys_vec *phys); -- 2.47.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with vfio_pci_core_get_iomap() 2026-04-30 10:03 ` [PATCH v3 2/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with vfio_pci_core_get_iomap() Matt Evans @ 2026-04-30 20:13 ` Alex Williamson 0 siblings, 0 replies; 8+ messages in thread From: Alex Williamson @ 2026-04-30 20:13 UTC (permalink / raw) To: Matt Evans Cc: Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas, Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy, Zhi Wang, kvm, linux-kernel, virtualization, alex On Thu, 30 Apr 2026 03:03:21 -0700 Matt Evans <mattev@meta.com> wrote: > Since "vfio/pci: Set up barmap in vfio_pci_core_enable()", the > resource request and iomap for the BARs was performed early, and > vfio_pci_core_setup_barmap() just checks those actions succeeded. > > Move this logic to a new helper that checks success and returns the > iomap address, replacing the various bare vdev->barmap[] lookups. > This maintains the error behaviour of the previous on-demand > vfio_pci_core_setup_barmap() scheme.> > Signed-off-by: Matt Evans <mattev@meta.com> > --- > drivers/vfio/pci/nvgrace-gpu/main.c | 17 +++++++------ > drivers/vfio/pci/vfio_pci_core.c | 11 ++++----- > drivers/vfio/pci/vfio_pci_rdwr.c | 37 +++++++---------------------- > drivers/vfio/pci/virtio/legacy_io.c | 13 +++++----- > include/linux/vfio_pci_core.h | 19 ++++++++++++++- > 5 files changed, 47 insertions(+), 50 deletions(-) > > diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c > index fa056b69f899..2f5ec60c15d9 100644 > --- a/drivers/vfio/pci/nvgrace-gpu/main.c > +++ b/drivers/vfio/pci/nvgrace-gpu/main.c > @@ -184,13 +184,11 @@ static int nvgrace_gpu_open_device(struct vfio_device *core_vdev) > > /* > * GPU readiness is checked by reading the BAR0 registers. > - * > - * ioremap BAR0 to ensure that the BAR0 mapping is present before > - * register reads on first fault before establishing any GPU > - * memory mapping. > + * The BAR map was just set up by vfio_pci_core_enable() and, > + * although the readiness check checks validity of the BAR0 > + * map, assert early that the map was successful: > */ > - ret = vfio_pci_core_setup_barmap(vdev, 0); > - if (ret) > + if (IS_ERR(vfio_pci_core_get_iomap(vdev, 0))) > goto error_exit; > > if (nvdev->resmem.memlength) { > @@ -265,6 +263,7 @@ static int > nvgrace_gpu_check_device_ready(struct nvgrace_gpu_pci_core_device *nvdev) > { > struct vfio_pci_core_device *vdev = &nvdev->core_device; > + void __iomem *io; > int ret; > > lockdep_assert_held_read(&vdev->memory_lock); > @@ -275,7 +274,11 @@ nvgrace_gpu_check_device_ready(struct nvgrace_gpu_pci_core_device *nvdev) > if (!__vfio_pci_memory_enabled(vdev)) > return -EIO; > > - ret = nvgrace_gpu_wait_device_ready(vdev->barmap[0]); > + io = vfio_pci_core_get_iomap(vdev, 0); > + if (IS_ERR(io)) > + return PTR_ERR(io); > + > + ret = nvgrace_gpu_wait_device_ready(io); I suspect the preference would be to test: if (IS_ERR(vfio_pci_core_get_iomap(vdev, 0))) goto error_exit; in nvgrace_gpu_open_device(), then just use: ret = nvgrace_gpu_wait_device_ready(vfio_pci_core_get_iomap(vdev, 0); here. > if (ret) > return ret; > > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c > index eab4f2626b39..feaf894ac118 100644 > --- a/drivers/vfio/pci/vfio_pci_core.c > +++ b/drivers/vfio/pci/vfio_pci_core.c > @@ -1760,7 +1760,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma > struct pci_dev *pdev = vdev->pdev; > unsigned int index; > u64 phys_len, req_len, pgoff, req_start; > - int ret; > + void __iomem *bar_io; > > index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); > > @@ -1794,12 +1794,11 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma > return -EINVAL; > > /* > - * Even though we don't make use of the barmap for the mmap, > - * we need to request the region and the barmap tracks that. > + * Ensure the BAR resource region is reserved for use. > */ > - ret = vfio_pci_core_setup_barmap(vdev, index); > - if (ret) > - return ret; > + bar_io = vfio_pci_core_get_iomap(vdev, index); > + if (IS_ERR(bar_io)) > + return PTR_ERR(bar_io); > > vma->vm_private_data = vdev; > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c > index f66ad3d96481..7f14dd46de17 100644 > --- a/drivers/vfio/pci/vfio_pci_rdwr.c > +++ b/drivers/vfio/pci/vfio_pci_rdwr.c > @@ -198,26 +198,6 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, > } > EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw); > > -int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar) > -{ > - /* > - * The barmap is set up in vfio_pci_core_enable(). Callers > - * use this function to check that the BAR resources are > - * requested or that the pci_iomap() was done. > - */ > - if (bar < 0 || bar >= PCI_STD_NUM_BARS) > - return -EINVAL; > - > - /* Did vfio_pci_core_map_bars() set it up yet? */ > - if (!vdev->barmap[bar]) > - return -ENODEV; > - > - if (IS_ERR(vdev->barmap[bar])) > - return PTR_ERR(vdev->barmap[bar]); > - return 0; > -} > -EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap); > - > ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf, > size_t count, loff_t *ppos, bool iswrite) > { > @@ -269,13 +249,11 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf, > */ > max_width = VFIO_PCI_IO_WIDTH_4; > } else { > - int ret = vfio_pci_core_setup_barmap(vdev, bar); > - if (ret) { > - done = ret; > + io = vfio_pci_core_get_iomap(vdev, bar); > + if (IS_ERR(io)) { > + done = PTR_ERR(io); > goto out; > } > - > - io = vdev->barmap[bar]; > } > > if (bar == vdev->msix_bar) { > @@ -430,6 +408,7 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset, > loff_t pos = offset & VFIO_PCI_OFFSET_MASK; > int ret, bar = VFIO_PCI_OFFSET_TO_INDEX(offset); > struct vfio_pci_ioeventfd *ioeventfd; > + void __iomem *io; > > /* Only support ioeventfds into BARs */ > if (bar > VFIO_PCI_BAR5_REGION_INDEX) > @@ -447,9 +426,9 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset, > if (count == 8) > return -EINVAL; > > - ret = vfio_pci_core_setup_barmap(vdev, bar); > - if (ret) > - return ret; > + io = vfio_pci_core_get_iomap(vdev, bar); > + if (IS_ERR(io)) > + return PTR_ERR(io); > > mutex_lock(&vdev->ioeventfds_lock); > > @@ -486,7 +465,7 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset, > } > > ioeventfd->vdev = vdev; > - ioeventfd->addr = vdev->barmap[bar] + pos; > + ioeventfd->addr = io + pos; > ioeventfd->data = data; > ioeventfd->pos = pos; > ioeventfd->bar = bar; > diff --git a/drivers/vfio/pci/virtio/legacy_io.c b/drivers/vfio/pci/virtio/legacy_io.c > index 1ed349a55629..c868b2177310 100644 > --- a/drivers/vfio/pci/virtio/legacy_io.c > +++ b/drivers/vfio/pci/virtio/legacy_io.c > @@ -299,19 +299,18 @@ int virtiovf_pci_ioctl_get_region_info(struct vfio_device *core_vdev, > static int virtiovf_set_notify_addr(struct virtiovf_pci_core_device *virtvdev) > { > struct vfio_pci_core_device *core_device = &virtvdev->core_device; > - int ret; > + void __iomem *io; > > /* > * Setup the BAR where the 'notify' exists to be used by vfio as well > * This will let us mmap it only once and use it when needed. > */ > - ret = vfio_pci_core_setup_barmap(core_device, > - virtvdev->notify_bar); > - if (ret) > - return ret; > + io = vfio_pci_core_get_iomap(core_device, > + virtvdev->notify_bar); > + if (IS_ERR(io)) > + return PTR_ERR(io); > > - virtvdev->notify_addr = core_device->barmap[virtvdev->notify_bar] + > - virtvdev->notify_offset; > + virtvdev->notify_addr = io + virtvdev->notify_offset; > return 0; > } > > diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h > index 2ebba746c18f..5598071c5ea3 100644 > --- a/include/linux/vfio_pci_core.h > +++ b/include/linux/vfio_pci_core.h > @@ -188,7 +188,6 @@ int vfio_pci_core_match_token_uuid(struct vfio_device *core_vdev, > int vfio_pci_core_enable(struct vfio_pci_core_device *vdev); > void vfio_pci_core_disable(struct vfio_pci_core_device *vdev); > void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev); > -int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar); > pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev, > pci_channel_state_t state); > ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, > @@ -234,6 +233,24 @@ static inline bool is_aligned_for_order(struct vm_area_struct *vma, > !IS_ALIGNED(pfn, 1 << order))); > } > > +/* > + * Returns a BAR's iomap base, or an ERR_PTR() if, for example, the > + * BAR isn't valid, its resource wasn't acquired, or its iomap > + * failed. > + */ > +static inline void __iomem __must_check * > +vfio_pci_core_get_iomap(struct vfio_pci_core_device *vdev, int bar) > +{ > + if (bar < 0 || bar >= PCI_STD_NUM_BARS) > + return ERR_PTR(-EINVAL); > + > + /* Did vfio_pci_core_map_bars() set it up yet? */ > + if (!vdev->barmap[bar]) > + return ERR_PTR(-ENODEV); > + > + return vdev->barmap[bar]; > +} > + Regarding the previously exported symbol, if the concern is that it was a GPL symbol and now it's a static inline, it's not doing anything that couldn't easily be open coded, so I don't see an issue. Thanks, Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] vfio/pci: Check BAR resources before exporting a DMABUF 2026-04-30 10:03 [PATCH v3 0/3] vfio/pci: Request resources and map BARs at enable time Matt Evans 2026-04-30 10:03 ` [PATCH v3 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable() Matt Evans 2026-04-30 10:03 ` [PATCH v3 2/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with vfio_pci_core_get_iomap() Matt Evans @ 2026-04-30 10:03 ` Matt Evans 2026-04-30 20:13 ` Alex Williamson 2 siblings, 1 reply; 8+ messages in thread From: Matt Evans @ 2026-04-30 10:03 UTC (permalink / raw) To: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas Cc: Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy, Zhi Wang, kvm, linux-kernel, virtualization A DMABUF exports access to BAR resources and, although they are requested at startup time, we need to ensure they really were reserved before exporting. Otherwise, it's possible to access unreserved resources through the export. Add a check to the DMABUF-creation 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 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index f87fd32e4a01..3bc7d850e258 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -244,9 +244,11 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, return -EINVAL; /* - * For PCI the region_index is the BAR number like everything else. + * For PCI the region_index is the BAR number like everything + * else. Check that PCI resources have been claimed for it. */ - if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX) + if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX || + !IS_ERR(vfio_pci_core_get_iomap(vdev, get_dma_buf.region_index))) return -ENODEV; dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges, -- 2.47.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] vfio/pci: Check BAR resources before exporting a DMABUF 2026-04-30 10:03 ` [PATCH v3 3/3] vfio/pci: Check BAR resources before exporting a DMABUF Matt Evans @ 2026-04-30 20:13 ` Alex Williamson 0 siblings, 0 replies; 8+ messages in thread From: Alex Williamson @ 2026-04-30 20:13 UTC (permalink / raw) To: Matt Evans Cc: Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas, Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy, Zhi Wang, kvm, linux-kernel, virtualization, alex On Thu, 30 Apr 2026 03:03:22 -0700 Matt Evans <mattev@meta.com> wrote: > A DMABUF exports access to BAR resources and, although they are > requested at startup time, we need to ensure they really were reserved > before exporting. Otherwise, it's possible to access unreserved > resources through the export. > > Add a check to the DMABUF-creation 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 | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c > index f87fd32e4a01..3bc7d850e258 100644 > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c > @@ -244,9 +244,11 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, > return -EINVAL; > > /* > - * For PCI the region_index is the BAR number like everything else. > + * For PCI the region_index is the BAR number like everything > + * else. Check that PCI resources have been claimed for it. > */ > - if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX) > + if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX || > + !IS_ERR(vfio_pci_core_get_iomap(vdev, get_dma_buf.region_index))) Polarity of the test is wrong, should just be IS_ERR(). It would be good practice here to front-load the Fixes: patches in your series. I'd suggest making this patch #2, using the existing setup_barmap API, then include it in the conversion to iomap in patch #3. Thanks, Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-05 16:40 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-30 10:03 [PATCH v3 0/3] vfio/pci: Request resources and map BARs at enable time Matt Evans 2026-04-30 10:03 ` [PATCH v3 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable() Matt Evans 2026-04-30 20:13 ` Alex Williamson 2026-05-05 16:40 ` Matt Evans 2026-04-30 10:03 ` [PATCH v3 2/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with vfio_pci_core_get_iomap() Matt Evans 2026-04-30 20:13 ` Alex Williamson 2026-04-30 10:03 ` [PATCH v3 3/3] vfio/pci: Check BAR resources before exporting a DMABUF Matt Evans 2026-04-30 20:13 ` Alex Williamson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox