Re: [PATCH V1] accel/amxdna: fix page-insertion errors in amdxdna_insert_pages()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Incompatible VMA flags modification replaces VM_PFNMAP with VM_MIXEDMAP, creating a locally exploitable kernel panic via BUG_ON.
- [High] Eager page table population inside the mmap callback bypasses interval tree tracking, risking Use-After-Free.
--

--- Patch [1]: [PATCH V1] accel/amxdna: fix page-insertion errors in amdxdna_insert_pages() ---
commit c41b85178d2bef0390b7acad007274b457c13742
Author: Lizhi Hou <lizhi.hou@amd.com>

accel/amxdna: fix page-insertion errors in amdxdna_insert_pages()

This commit prevents a reference count underflow when error paths in
amdxdna_insert_pages() prematurely close a VMA. It replaces hard error
returns with a deferred-fault approach that keeps the VMA alive and retries
page insertion later.

> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index df236a9d8a0a4..493f34473266e 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
[ ... ]
> @@ -457,8 +474,7 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
>  				      &num_pages);

[Severity: Critical]
This is a pre-existing issue, but does replacing VM_PFNMAP with VM_MIXEDMAP
create a locally exploitable kernel panic?

Just above this line, vm_flags_mod() sets VM_MIXEDMAP and clears VM_PFNMAP.

If an unprivileged userspace process mmaps a non-imported GEM object and then
calls madvise(MADV_DONTNEED), the PTEs will be successfully cleared because
VM_MIXEDMAP allows this (unlike VM_PFNMAP).

When userspace subsequently accesses the memory, drm_gem_shmem_fault() handles
the page fault and attempts to map the backing shmem page via vmf_insert_pfn()
which calls vmf_insert_pfn_prot().

Because the backing shmem page is normal system memory (pfn_valid(pfn) is
true) and the VMA now has VM_MIXEDMAP set, won't this predictably trigger the
explicit assertion BUG_ON((vma->vm_flags & VM_MIXEDMAP) && pfn_valid(pfn)),
crashing the system?

[Severity: High]
This is also a pre-existing issue, but does eagerly populating PTEs inside
the mmap callback bypass interval tree tracking?

At the time the .mmap callback executes, the new VMA is not yet linked into
the backing file's interval tree (mapping->i_mmap). The core mm links it
via vma_link_file() only after the .mmap callback returns successfully.

If a dma-buf exporter concurrently evicts the memory and calls
unmap_mapping_range(), won't the invalidation miss this unlinked VMA and
fail to shoot down the eagerly populated PTEs, leading to a use-after-free?

>  		if (ret) {
>  			XDNA_ERR(xdna, "Failed insert pages %d", ret);
> -			vma->vm_ops->close(vma);
> -			return ret;
> +			amdxdna_mark_mapp_invalid(abo, vma);
>  		}
>  
>  		return 0;
> @@ -478,9 +494,9 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
>  		fault_ret = handle_mm_fault(vma, vma->vm_start + offset,
>  					    FAULT_FLAG_WRITE, NULL);

[Severity: High]
This isn't a bug introduced by this patch, but does this eager population for
dma-buf imports have the same interval tree tracking bypass issue?

Since the VMA is not yet linked into the interval tree during the .mmap
callback, an invalidation from the exporter might miss these eagerly
populated PTEs, resulting in a use-after-free.

>  		if (fault_ret & VM_FAULT_ERROR) {
> -			vma->vm_ops->close(vma);
>  			XDNA_ERR(xdna, "Fault in page failed");
> -			return -EFAULT;
> +			amdxdna_mark_mapp_invalid(abo, vma);
> +			break;
>  		}
>  
>  		offset += PAGE_SIZE;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723074256.2435143-1-lizhi.hou@amd.com?part=1