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

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

[Lizhi Hou]

Two error paths in amdxdna_insert_pages() called vma->vm_ops->close(vma)
before returning an error code to the caller.  This is incorrect:
amdxdna_gem_obj_mmap() registers an HMM interval notifier before calling
amdxdna_insert_pages(), and on a hard error it jumps to hmm_unreg to undo
that registration.  Calling vm_ops->close() manually — which drops the
shmem pages_pin_count and the GEM object reference that backs the VMA —
before the mmap syscall has even returned causes those resources to be
released while the VMA is still alive.  The kernel VMA teardown will call
vm_ops->close() a second time when the process later unmaps the range,
producing a reference count underflow.

Replace both hard-error returns with a deferred-fault approach that keeps
the VMA alive and retries page insertion through the HMM range-fault path.

Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index d6fe6fb41286..aed110ad1c1e 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -435,6 +435,23 @@ static void amdxdna_gem_dev_obj_free(struct drm_gem_object *gobj)
 	amdxdna_gem_destroy_obj(abo);
 }
 
+static void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo,
+				      struct vm_area_struct *vma)
+{
+	struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
+	struct amdxdna_umap *mapp;
+
+	down_write(&xdna->notifier_lock);
+	abo->mem.map_invalid = true;
+	list_for_each_entry(mapp, &abo->mem.umap_list, node) {
+		if (compare_range(mapp, vma->vm_mm, vma->vm_start, vma->vm_end)) {
+			mapp->invalid = true;
+			break;
+		}
+	}
+	up_write(&xdna->notifier_lock);
+}
+
 static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
 				struct vm_area_struct *vma)
 {
@@ -456,8 +473,7 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
 				      &num_pages);
 		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;
@@ -477,9 +493,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);
 		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;
-- 
2.34.1

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

[sashiko-bot]

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

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

[Max Zhen]

On 7/23/2026 Thu 00:42, Lizhi Hou wrote:
> Two error paths in amdxdna_insert_pages() called vma->vm_ops->close(vma)
> before returning an error code to the caller.  This is incorrect:
> amdxdna_gem_obj_mmap() registers an HMM interval notifier before calling
> amdxdna_insert_pages(), and on a hard error it jumps to hmm_unreg to undo
> that registration.  Calling vm_ops->close() manually — which drops the
> shmem pages_pin_count and the GEM object reference that backs the VMA —
> before the mmap syscall has even returned causes those resources to be
> released while the VMA is still alive.  The kernel VMA teardown will call
> vm_ops->close() a second time when the process later unmaps the range,
> producing a reference count underflow.
> 
> Replace both hard-error returns with a deferred-fault approach that keeps
> the VMA alive and retries page insertion through the HMM range-fault path.
> 
> Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@amd.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 24 ++++++++++++++++++++----
>   1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index d6fe6fb41286..aed110ad1c1e 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -435,6 +435,23 @@ static void amdxdna_gem_dev_obj_free(struct drm_gem_object *gobj)
>   	amdxdna_gem_destroy_obj(abo);
>   }
>   
> +static void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo,
> +				      struct vm_area_struct *vma)
> +{
> +	struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
> +	struct amdxdna_umap *mapp;
> +
> +	down_write(&xdna->notifier_lock);
> +	abo->mem.map_invalid = true;
> +	list_for_each_entry(mapp, &abo->mem.umap_list, node) {
> +		if (compare_range(mapp, vma->vm_mm, vma->vm_start, vma->vm_end)) {
> +			mapp->invalid = true;
> +			break;
> +		}
> +	}
> +	up_write(&xdna->notifier_lock);
> +}
> +
>   static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
>   				struct vm_area_struct *vma)
>   {
> @@ -456,8 +473,7 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
>   				      &num_pages);
>   		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;
> @@ -477,9 +493,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);
>   		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;