On 09/10/2026 1:21 am, Karl Mehltretter wrote:
> Diederik reported failed video-buffer imports on RK3588 with
> DMABUF_DEBUG. The proposed warn-only mode [1] confirmed CPU-side
> attachment access in rockchip_gem_iommu_map() during Sway video resizing.
>
> This RFC keeps Rockchip's private scanout domain. Patch 1 copies live
> iommu-dma mappings into another domain. Patch 2 uses it for PRIME
> imports. Unsupported inputs keep the old page-based path, which remains
> unfixed under strict DMABUF_DEBUG.
>
> Rob's MSM approach assumes direct DMA [2]. Rockchip's attachment instead
> provides IOVAs from the VOP's default domain, which cannot be mapped
> unchanged into the private domain. Each import retains both mappings
> and adds one reverse lookup per 4 KiB.
>
> Christian rejected reverse translation in the earlier MSM proposal [3].
> Robin NAKed exporting iommu_get_dma_domain() [4]. Keeping the helper in
> dma-iommu.c does not resolve the physical-address objection.
And if I'd had the context of the whole series, I would have said what I
can see now, that the entire premise is fundamentally wrong,
irrespective of abusing the internal helper or not.
> Sharing the first VOP's default DMA domain, as Exynos does, would avoid
> the translation but also change native-buffer mapping.
>
> Is retaining the private domain and reverse-translating the DMA mappings
> an acceptable direction? If so, I'll address the remaining limitations
> before posting a non-RFC version.
No. If a driver has attached the device to its own unmanaged IOMMU
domain then it is using that domain, not the default domain, and thus
has even less reason to go poking at the default domain than usual
(where the "usual" is tenuous enough in itself). In this situation DMA
mapping only needs to take care of non-coherent cache maintenance, and
32-bit ARM is actually the better example here.
The fact that iommu-dma does a load of unnecessary work and returns a
bogus DMA address just to still get the cache maintenance as a
side-effect is a hideous inefficiency (which folks have complained about
before...) and absolutely should not be relied upon. It needs to go
away. There were reasons why in the original iommu_dma_ops design it was
rather impractical to do better (in fact iommu_get_dma_domain() itself
is largely just a hack around some of those limitations), but since
b67483b3c44e ("iommu/dma: Centralise iommu_setup_dma_ops()") and
particularly b5c58b2fdc42 ("dma-mapping: direct calls for dma-iommu"),
it now really could and should be cleaned up - it just needs something
slightly different from the standard dma-direct behaviour, as for this
case we need to ignore the DMA mask and any bouncing conditions.
Thanks,
Robin.
> Base: mainline 602042bf29f6. The warn-only RFC is not a prerequisite.
> No stable backport requested. Strict-by-default DMABUF_DEBUG took effect
> in v7.3-rc4.
>
> Testing (builds and QEMU only):
>
> - W=1 object and stub builds passed on arm64, ARM32 with/without LPAE,
> x86-64 GCC/Clang, i386, s390, RISC-V and UML, including dynamic SWIOTLB.
> - Rockchip strict/warn A/B reproduced the control failure and warning.
> Treatments checked every page and byte in 83 imports each. Primary
> buffers had 1/127/507 DMA segments under the default 64 KiB limit.
> - SMMUv3 strict/lazy tests, missing-source, bounds and rollback tests
> passed. Real bounced attachments exercised alignment and pool
> rejection with no target mapping or DMA.
> - Invalid forced-SWIOTLB Rockchip provider runs are excluded. Expected
> segment-limit and unsupported-fallback diagnostics remain.
>
> The rig uses a custom Rockchip IOMMU model and the real GEM callback in a
> test module, not VOP2, Sway or a real exporter. RK3588 hardware, two-VOP
> and 32-bit ARM runtime testing are outstanding.
>
> Hardware testing is welcome, particularly with Diederik's Sway resize
> workload. Compare control and both patches on the same base/config,
> first with strict DMABUF_DEBUG, then with the warn-only RFC on both.
> Please report full dmesg, config, exporter and visible display problems.
> Keep a known-good boot kernel. DMABUF_DEBUG=n testing is welcome too.
>
> Developed and tested with LLM assistance.
>
> [1] https://lore.kernel.org/r/20261005064133.7305-1-kmehltretter@gmail.com/
> [2] https://lore.kernel.org/r/20261006131000.81501-1-robin.clark@oss.qualcomm.c…
> [3] https://lore.kernel.org/r/bd4e5ece-1358-4e0b-bb04-ba9de62d26f6@amd.com/
> [4] https://lore.kernel.org/r/47bf9a4c-2a47-4e93-bcdf-8d953c9a5ab8@arm.com/
>
> Reports:
> https://lore.kernel.org/r/DLR74W1U9YPC.375IK0HOYHDIG@cknow-tech.com/
> https://lore.kernel.org/r/DLYLA3WQNN3X.3FB9MO8ZWBQ5@cknow-tech.com/
>
> Karl Mehltretter (2):
> iommu: Add iommu_map_sgtable_dma()
> drm/rockchip: Map imported buffers from DMA addresses
>
> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 19 ++-
> drivers/iommu/dma-iommu.c | 131 ++++++++++++++++++++
> include/linux/iommu.h | 12 ++
> 3 files changed, 157 insertions(+), 5 deletions(-)
>
>
> base-commit: 602042bf29f6efde39cfb5fdd9289bf4854bc0c5
On 10/5/26 08:41, Karl Mehltretter wrote:
> With DMABUF_DEBUG, dma_buf_map_attachment() hands the importer a copy
> of the sg_table that keeps only sg_dma_address() and sg_dma_len(). The
> dma_flags (SG_DMA_BUS_ADDRESS, SG_DMA_SWIOTLB) are dropped.
>
> The flags describe the DMA side of an entry, which is the side
> importers may use. Copy them as well.
>
> This is not a bug fix. No importer reads the flags today. Their only
> readers are dma-iommu, dma-direct and iommu_map_sg(), which are not
> supposed to see an importer's copy at all. The warn mode added in the
> next patch keeps its marker in dma_flags and lets importers that do
> get there continue. They should then see the same flags as without
> DMABUF_DEBUG.
>
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter(a)gmail.com>
> ---
>
> Notes:
> Built alone on x86_64 with NEED_SG_DMA_FLAGS=y (dma-buf.o, W=1). The
> KUnit test in patch 3 checks the flags in the copy. It passes on x86_64
> under QEMU in strict and in warn mode.
>
> drivers/dma-buf/dma-buf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 4c9add51f9ef..b3d311acb883 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -904,6 +904,10 @@ static int dma_buf_wrap_sg_table(struct sg_table **sg_table)
> sg_assign_page(to_sg, NULL);
> sg_dma_address(to_sg) = sg_dma_address(from_sg);
> sg_dma_len(to_sg) = sg_dma_len(from_sg);
> +#ifdef CONFIG_NEED_SG_DMA_FLAGS
> + /* the flags describe the DMA side, e.g. SG_DMA_BUS_ADDRESS */
> + to_sg->dma_flags = from_sg->dma_flags;
We should probably have a WARN_ON_ONCE(sg_dma_is_swiotlb(from_sg)) here as well since some importers/exporters doesn't realize that this combination will usually not work.
Apart from that looks good to me.
Regards,
Christian.
> +#endif
> to_sg = sg_next(to_sg);
> }
>
On Wed, Oct 07, 2026 at 02:02:31PM +0200, Diederik de Haas wrote:
> [ 1091.981354] rc rc3: two consecutive events of type space
> [ 1104.670513] input: EDIFIER e235 (AVRCP) as /devices/virtual/input/input12
> [ 1172.090231] devfreq fb000000.gpu: Couldn't update frequency transition information.
> [ 1189.403278] devfreq fb000000.gpu: Couldn't update frequency transition information.
> [ 1206.309295] input: EDIFIER e235 (AVRCP) as /devices/virtual/input/input13
> [ 1268.968696] DMA-BUF: importer used the CPU side of an exporter's sg_table
This is a nice stack trace, is that the point of this series?
> [ 1268.968710] CPU: 6 UID: 1000 PID: 29025 Comm: sway Not tainted 7.3-rc6+unreleased-arm64-cknow #1 PREEMPTLAZY Debian 7.3~rc6-3
> [ 1268.968715] Hardware name: FriendlyElec NanoPC-T6 Plus (DT)
> [ 1268.968717] Call trace:
> [ 1268.968719] show_stack+0x20/0x38 (C)
> [ 1268.968726] dump_stack_lvl+0x60/0x80
> [ 1268.968730] sg_dmabuf_cpu_access_warn.part.0+0x24/0x30
> [ 1268.968734] sg_dmabuf_cpu_access_warn+0x34/0x38
> [ 1268.968739] iommu_map_sg+0xc8/0x1e0
> [ 1268.968745] rockchip_gem_iommu_map+0x8c/0x128 [rockchipdrm]
> [ 1268.968757] rockchip_gem_prime_import_sg_table+0x58/0x160 [rockchipdrm]
> [ 1268.968761] drm_gem_prime_import_dev+0xa8/0x1d0 [drm]
> [ 1268.968776] drm_gem_prime_fd_to_handle+0x1a4/0x280 [drm]
So.. This is the exact same thing I need for iommufd.
rockchip is managing its own iommu domain and you cannot map to an
iommu domain without using a physical address.
Of course it is *completely* illegal to call iommu_map_sgtable() in the
importer side of a dmabuf.
static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
{
[..]
ret = iommu_map_sgtable(private->domain, rk_obj->dma_addr, rk_obj->sgt,
prot);
Jason
On Mon, Oct 05, 2026 at 08:41:32AM +0200, Karl Mehltretter wrote:
> static inline struct page *sg_page(struct scatterlist *sg)
> {
> #ifdef CONFIG_DEBUG_SG
> BUG_ON(sg_is_chain(sg));
> #endif
> + sg_dmabuf_cpu_access_check(sg);
> return (struct page *)((sg)->page_link & ~SG_PAGE_LINK_MASK);
> }
I'm not sure I understand the overall intention here, I get what this
patch does, but no distro could turn this on by default when it
touches *everyone* using scatterlist in a performance sensitive spot,
and anyone doing testing can use the existing option - so what is the
point?
I certainly don't like this patch, and I don't like the word "DMABUF"
in the scatterlist at all. If we want to add something it should be a
general mechanism under DEBUG_SG that allows anyone to 'hide' the CPU
list from any future access.
Jason
Hi all,
A fix for DMABUF's file-release path, to set dmabuf->file = NULL when
the struct file itself is being freed and avoid a stale pointer
lingering. Previously it could be assumed that dmabuf->file was
always valid until .release was called, but commit 4ab59c3c638c6
("dma-buf: Move dma_buf_release() from fops to dentry_ops") separated
dentry release (calls .release) from file release (can happen before
.release). Setting the pointer to NULL clearly delimits the period in
which it points to a valid struct file.
Raw dmabuf->file access is bad news unless the file is known to have a
reference held (for example, during export). A single-threaded
problem scenario is a dentry held open (e.g. using O_PATH) but the
file closed; .release is not called, so the exporter was unaware that
dmabuf->file became stale. Exporter activities using dmabuf->file
then led to a UAF. Or, threads racing concurrent access with a file
closing.
VFIO uses get_file_active(&dmabuf->file), which is a UAF when the
pointer goes stale. Alex Mastro has created a VFIO-centric reproducer
[1], which shows up with KASAN.
To fix, dmabuf->file is either valid or NULL. get_file_active() is
then safe and a recommended way to access dmabuf->file without an
existing reference. The only possible outcomes are for it to get a
reference to the file, to observe a valid pointer but the file in the
DEAD state (being closed, fail), or NULL (fail).
Patch [2/2] sets the pointer NULL when the file closes, which
get_file_active() handles correctly.
vmwgfx was identified as doing a problematic dmabuf->file dereference.
Although already a UAF, making dmabuf->file = NULL turns it into an
oops. So, the paired patch [1/2] correspondingly converts vmwgfx to
use get_file_active() too.
Added comments to highlight that dmabuf->file could go away before
.release, and to use get_file_active() if a reference isn't otherwise
already held.
Thanks,
Matt
[1]: https://github.com/opsound/vfio-dmabuf-lab/blob/main/tests/vfio_dmabuf_opat…
"v1" was the single "dma-buf: Annul dmabuf->file on file release"
patch:
https://lore.kernel.org/all/f8efaabd-c06e-4f04-8cfa-489c148e37ba@ozlabs.org/
Matt Evans (2):
drm/vmwgfx: Use get_file_active() for dmabuf->file
dma-buf: Annul dmabuf->file on file release
drivers/dma-buf/dma-buf.c | 21 ++++++++++++++++++---
drivers/gpu/drm/vmwgfx/ttm_object.c | 2 +-
include/linux/dma-buf.h | 4 +++-
3 files changed, 22 insertions(+), 5 deletions(-)
--
2.47.3
Hi Zack,
On 07/10/2026 16:24, Zack Rusin wrote:
> On Tue, Oct 6, 2026 at 3:15 PM Matt Evans <matt(a)ozlabs.org> wrote:
>>
>> The dmabuf release path is split between file and dentry release:
>> dma_buf_file_release() is called shortly before the file is freed, and
>> dma_buf_release() calls an exporter's dmabuf->ops->release when the
>> dentry is freed. However, the dentry can outlive the file (for
>> example, if opened with O_PATH), meaning .release might be called some
>> time after the file is freed.
>>
>> This presents a window, when closing a DMABUF file, in which
>> dmabuf->file points to freed memory yet .release op has not yet been
>> called.
>>
>> For VFIO, if a buffer's .release has not been called it's considered
>> still active and subject to move/cleanup. If so, it attempts to
>> get_file_active() on dmabuf->file: a file close with dentry held open
>> will call this function with a stale pointer, a UAF.
>>
>> To make this pattern safe, set dmabuf->file to NULL in
>> dma_buf_file_release() to reflect that the associated file is now dead
>> even if the DMABUF is not yet gone. A get_file_active() ... fput()
>> sequence concurrent with a file close will only execute as one of:
>>
>> - Gets the file before file_ref_put() (dmabuf->file valid)
>> - Observes dmabuf->file pointing to a file, but it's DEAD (no file)
>> - Observes dmabuf->file = NULL (no file)
>>
>> Originally, drivers could assume dmabuf->file was valid until .release
>> was called from fops->release. This assumption was no longer valid
>> after 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to
>> dentry_ops"), which moved the callback to the dentry release (by which
>> point the file might have been freed). With this commit, drivers must
>> still consider that dmabuf->file could be NULL before .release.
>>
>> Fixes: 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to dentry_ops")
>> Signed-off-by: Matt Evans <matt(a)ozlabs.org>
>> ---
>>
>> Hi,
>>
>> This issue was found (by Claude Opus 5.5) in the context of VFIO's
>> DMABUF export path. VFIO iterates live DMABUFs with a
>> get_file_active()/fput() block, which now becomes safe if the file is
>> closed (and memory freed!) yet DMABUF .release hasn't yet occurred.
>>
>> However, there are a couple of other places that directly use
>> dmabuf->file and seem able to race a closing file (i.e. without holding
>> the file reference)? If this is so, they'd be a UAF today; with this
>> patch that goes away, but instead of a stale pointer dmabuf->file could
>> be NULL:
>>
>> 1. drivers/gpu/drm/vmwgfx/ttm_object.c:get_dma_buf_unless_doomed()
>>
>> file_ref_get(&dmabuf->file->f_ref); on a non-refcounted DMABUF
>
> Yes, that's a real UAF today. vmwgfx relied on the exporter's
> dma_buf_ops.release running inside __fput() before file_free(), which
> stopped being true with 4ab59c3c638c and we missed it.
>
> But this patch on its own makes vmwgfx worse. In the ordinary case,
> where closing the file drops the last dentry reference, a close racing
> PRIME_HANDLE_TO_FD is safe today: ttm_prime_dmabuf_release() runs from
> the same __fput() and blocks on prime->mutex, keeping the file
> allocated, so file_ref_get() just fails on the dead refcount. With
> this patch dmabuf->file is NULL in that window and we oops. So this
> shouldn't go to stable without a vmwgfx fix.
>
> For vmwgfx we'd probably need this one line change in
> drivers/gpu/drm/vmwgfx/ttm_object.c to accompany this patch:
> @@ -471,7 +475,7 @@ void ttm_object_device_release(struct
> ttm_object_device **p_tdev)
> */
> static bool __must_check get_dma_buf_unless_doomed(struct dma_buf *dmabuf)
> {
> - return file_ref_get(&dmabuf->file->f_ref);
> + return get_file_active(&dmabuf->file) != NULL;
> }
>
> under prime->mutex, which keeps the struct dma_buf alive. It needs the
> NULL store from this patch to fix the existing UAF, since otherwise
> the recheck can accept a recycled file through the unchanged stale
> pointer.
Thanks for confirming! Ah, so get_file_active() is good here, cheers.
I'll repost as a 2-part series to first include this in vmwgfx and
second add the anNULL.
>> The commit message of 90ee6ed776c0 ("fs: port files to file_ref") hints
>> this might be more subtle than replacing it with a get_file() variant
>> (so as to accept a NULL file *).
>>
>> 2. drivers/gpu/drm/i915/gvt/dmabuf.c:intel_vgpu_get_dmabuf()
>>
>> gvt_dbg_dpy(... file_count(dmabuf->file) ...);
FWIW aside, I re-read this, and there does appear to be a ref held
(dma_buf_export() has just happened, and dma_buf_fd() generated) so
dmabuf->file is stable at that point.
>> Respective vmwgfx & i915 maintainers, what is your view?
>> Or, indeed, if anyone sees any other questionable uses of dmabuf->file.
>>
>> Thanks,
>>
>>
>> Matt
>>
>>
>> drivers/dma-buf/dma-buf.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 4c9add51f9ef..726639130477 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -193,11 +193,15 @@ static void dma_buf_release(struct dentry *dentry)
>>
>> static int dma_buf_file_release(struct inode *inode, struct file *file)
>> {
>> + struct dma_buf *dmabuf = file->private_data;
>> +
>> if (!is_dma_buf_file(file))
>> return -EINVAL;
>>
>> - __dma_buf_list_del(file->private_data);
>> + __dma_buf_list_del(dmabuf);
>>
>> + /* Must be observed by __get_file_rcu() before file_free() */
>> + smp_store_mb(dmabuf->file, NULL);
>
> I think Alex is right and dmabuf can be NULL here. An early if
> (!dmabuf) return 0" will fix that.
Yep, on it.
Many thanks,
Matt
PCI P2PDMA applies Request and Completion Redirect throughout both paths.
This misclassifies asymmetric and nested switches, and reports one answer
for every kind of TLP.
Three ACS controls act on TLP attributes the client chooses rather than
on the topology: Translation Blocking and Direct Translated P2P act on
a Request's Address Type, and Completion Redirect skips Completions carrying
Relaxed Ordering.
Evaluate each direction at the path divergence, decide every class from the
one walk, and treat a client with ATS enabled as translating unless its
driver declares per-mapping ATS.
This completes the P2PDMA side; dma-buf and mlx5 follow separately.
Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com>
---
Changes in v9:
- Removed dmabuf patches for now.
- Removed mlx5 per-mapping ATS declaration for now; it comes back with
the dmabuf patches.
- Link to v8: https://patch.msgid.link/20260928-fix-p2p-acs-v4-0-v8-0-404453b9c435@nvidia…
Changes in v8:
- Added extra Reviewed-by from Logan.
- Added patches to take care of ATS per-device vs. per-mapping option
- Link to v7: https://patch.msgid.link/20260920-fix-p2p-acs-v4-0-v7-0-ca0828ab697c@nvidia…
Changes in v7:
- Removed "Document pdev->p2pdma lifetime rules" patch, it gives nothing
after p2pmem fix.
- Split dmabuf patch.
- Tushar retested the series, so added his Tested-by.
- Added Logan's ROB tags and fixed minor documentation issues pointed
by him.
- Link to v6: https://lore.kernel.org/all/20260914-fix-p2p-acs-v4-0-v6-0-5ef07ec9ef06@nvi…
Changes in v6:
- Changed DMABUF to use callbacks and not directly stored pointer.
- Link to v5: https://patch.msgid.link/20260910-fix-p2p-acs-v4-0-v5-0-856087f63c0d@nvidia…
Changes in v5:
- Rebase on the posted fixes.
- Dropped tags from changed patches.
- Remove Egress Control Vector interpretation and coverage.
- Keep enabled Egress Control conservative as a Request redirect.
- Use pci_dbg()/dev_dbg() for diagnostics and drop the "debug" prefix.
- Removed code comments from "Document the pdev->p2pdma lifetime and RCU
rules" patch and reduced description to actual lifetime explanation.
- Added note that Linux assumes that TLPs are in strict-ordering and
untranslated.
- Added code to calculate p2p paths per-TLP type.
- Converted mlx5 to use that new proposed API.
- Link to https://patch.msgid.link/20260821-fix-p2p-acs-v4-0-v4-0-94426b96de73@nvidia…
Changes in v4:
- Reject ACS Violations and unreadable routing state instead of treating
them as host-bridge redirects
- Added Tested-by tags from Tushar Dave
- Added support to asymmetric ACS routing
- Limited redirect checks to the two ports at the path divergence
- Added standalone ACS routing diagnostics for hardware retesting
- Dropped " PCI: Account for Direct Translated P2P in ACS isolation checks" patch
- Link to v3: https://patch.msgid.link/20260811-fix-p2p-acs-v3-0-efc488ee7c03@nvidia.com
Changes in v3:
- Fixed pci_p2pdma_add_resource() error unwinding
- Made pdev->p2pdma teardown wait unconditionally for RCU readers
- Restricted pci_p2pmem_find_many() to pool-backed providers
- Documented the pdev->p2pdma lifetime and RCU rules
- Fixed calc_map_type_and_dist() handling of the verbose argument
- Required the ACS port and target to share a bus before indexing the
Egress Control Vector
- Gave pci_acs_enabled() and pci_acs_path_enabled() a scope, so the ACS
Direct Translated P2P rule no longer stops pci_enable_pasid() from
enabling PASID
- Dropped "Report ACS ports when the paths share no upstream bridge":
the mapping type cannot change without a shared upstream bridge, so
the pci=disable_acs_redir= hint was not actionable there and the ACS
walk only cost config space reads
- Folded the Request Redirect rule into pci_acs_rr_ineffective(), so
pci_acs_flags_enabled() and the Intel SPT PCH quirk share one copy
- Renamed pci_acs_egress_ctrl_set() to pci_acs_egress_ctrl_is_set(), it
reads the bit rather than setting it
- Reworded the blocked-path warning: ACS may also leave the direct route
indeterminate rather than blocked
- Added KUnit coverage for the shared-bus guard, a device with no ACS
capability and an unreadable ACS Control register
- Added the missing Fixes: tags, a second one on the
pci_p2pdma_add_resource() unwinding fix (the dangling devres action
dates to f58ef9d1d135) and one on the Egress Control isolation change
- Link to v2: https://patch.msgid.link/20260806-fix-p2p-acs-v2-0-0cec14812965@nvidia.com
Changes in v2:
- Added Logan's ROB tags
- Added commas in Documentation patch
- Link to v1: https://patch.msgid.link/20260802-fix-p2p-acs-v1-0-a7c5eb64fff6@nvidia.com
---
Leon Romanovsky (18):
PCI/P2PDMA: Document the TLP attribute assumptions
PCI/P2PDMA: Derive routing from directional ACS controls
PCI: Reject unreadable ACS controls in isolation checks
PCI/P2PDMA: Evaluate ACS controls at the path divergence
PCI/P2PDMA: Document directional ACS routing
PCI/P2PDMA: Collect the path's ACS controls before deciding
PCI/P2PDMA: Answer routing per TLP class
PCI/P2PDMA: Route Relaxed Ordering Completions directly
PCI/P2PDMA: Reject Translated Requests blocked by Translation Blocking
PCI/P2PDMA: Route Translated Requests under Direct Translated P2P
PCI/P2PDMA: Log detailed ACS routing diagnostics
PCI/P2PDMA: Add KUnit tests for the ACS routing decisions
PCI/P2PDMA: Test the ACS P2P routing walk
PCI: Add KUnit coverage for ACS isolation checks
PCI/P2PDMA: Document TLP-class routing
PCI/P2PDMA: Let a client declare that it selects ATS per mapping
PCI/P2PDMA: Evaluate the ATS path for clients with ATS enabled
PCI/P2PDMA: Test the routing of clients with ATS enabled
Documentation/admin-guide/kernel-parameters.txt | 15 +-
Documentation/driver-api/pci/p2pdma.rst | 80 +++
drivers/pci/Kconfig | 15 +
drivers/pci/Makefile | 1 +
drivers/pci/p2pdma.c | 719 ++++++++++++++++++--
drivers/pci/pci.c | 7 +-
drivers/pci/pci.h | 58 ++
drivers/pci/pci_acs_test.c | 860 ++++++++++++++++++++++++
drivers/pci/quirks.c | 6 +-
include/linux/pci-p2pdma.h | 12 +-
10 files changed, 1687 insertions(+), 86 deletions(-)
---
base-commit: 08dbfad3f5040f5bdb6c529da20d6d4e81fefd72
change-id: 20260821-fix-p2p-acs-v4-0-e72455e3a261
prerequisite-message-id: <20260830-batch-p2p-fixes-v1-0-5044e8dfbe2e(a)nvidia.com>
prerequisite-patch-id: 6b25c7fcf164cdfc14e9fac5b908d97fcf6509d7
prerequisite-patch-id: 0d083c281001365aae4b35544cf28891a6ab9a96
prerequisite-patch-id: bfd9dabf271f3cc9a3a61f46387d20c20311363d
prerequisite-patch-id: fad0275efc722830fc591509506c0a5e4f581073
prerequisite-patch-id: 0c83bee688fec1f6d1564654df7c630fa6a4a978
Best regards,
--
Leon Romanovsky <leonro(a)nvidia.com>
The dmabuf release path is split between file and dentry release:
dma_buf_file_release() is called shortly before the file is freed, and
dma_buf_release() calls an exporter's dmabuf->ops->release when the
dentry is freed. However, the dentry can outlive the file (for
example, if opened with O_PATH), meaning .release might be called some
time after the file is freed.
This presents a window, when closing a DMABUF file, in which
dmabuf->file points to freed memory yet .release op has not yet been
called.
For VFIO, if a buffer's .release has not been called it's considered
still active and subject to move/cleanup. If so, it attempts to
get_file_active() on dmabuf->file: a file close with dentry held open
will call this function with a stale pointer, a UAF.
To make this pattern safe, set dmabuf->file to NULL in
dma_buf_file_release() to reflect that the associated file is now dead
even if the DMABUF is not yet gone. A get_file_active() ... fput()
sequence concurrent with a file close will only execute as one of:
- Gets the file before file_ref_put() (dmabuf->file valid)
- Observes dmabuf->file pointing to a file, but it's DEAD (no file)
- Observes dmabuf->file = NULL (no file)
Originally, drivers could assume dmabuf->file was valid until .release
was called from fops->release. This assumption was no longer valid
after 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to
dentry_ops"), which moved the callback to the dentry release (by which
point the file might have been freed). With this commit, drivers must
still consider that dmabuf->file could be NULL before .release.
Fixes: 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to dentry_ops")
Signed-off-by: Matt Evans <matt(a)ozlabs.org>
---
Hi,
This issue was found (by Claude Opus 5.5) in the context of VFIO's
DMABUF export path. VFIO iterates live DMABUFs with a
get_file_active()/fput() block, which now becomes safe if the file is
closed (and memory freed!) yet DMABUF .release hasn't yet occurred.
However, there are a couple of other places that directly use
dmabuf->file and seem able to race a closing file (i.e. without holding
the file reference)? If this is so, they'd be a UAF today; with this
patch that goes away, but instead of a stale pointer dmabuf->file could
be NULL:
1. drivers/gpu/drm/vmwgfx/ttm_object.c:get_dma_buf_unless_doomed()
file_ref_get(&dmabuf->file->f_ref); on a non-refcounted DMABUF
The commit message of 90ee6ed776c0 ("fs: port files to file_ref") hints
this might be more subtle than replacing it with a get_file() variant
(so as to accept a NULL file *).
2. drivers/gpu/drm/i915/gvt/dmabuf.c:intel_vgpu_get_dmabuf()
gvt_dbg_dpy(... file_count(dmabuf->file) ...);
Respective vmwgfx & i915 maintainers, what is your view?
Or, indeed, if anyone sees any other questionable uses of dmabuf->file.
Thanks,
Matt
drivers/dma-buf/dma-buf.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 4c9add51f9ef..726639130477 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -193,11 +193,15 @@ static void dma_buf_release(struct dentry *dentry)
static int dma_buf_file_release(struct inode *inode, struct file *file)
{
+ struct dma_buf *dmabuf = file->private_data;
+
if (!is_dma_buf_file(file))
return -EINVAL;
- __dma_buf_list_del(file->private_data);
+ __dma_buf_list_del(dmabuf);
+ /* Must be observed by __get_file_rcu() before file_free() */
+ smp_store_mb(dmabuf->file, NULL);
return 0;
}
--
2.47.3