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

Lizhi Hou posted 1 patch 1 day, 13 hours ago
drivers/accel/amdxdna/amdxdna_gem.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
[PATCH V1] accel/amxdna: fix page-insertion errors in amdxdna_insert_pages()
Posted by Lizhi Hou 1 day, 13 hours ago
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()
Posted by Max Zhen 1 day, 3 hours ago

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;