[PATCH V1] accel/amdxdna: Fix VMA access race

[PATCH V1] accel/amdxdna: Fix VMA access race

[Lizhi Hou]

aie2_populate_range() and amdxdna_umap_release() access a saved VMA
pointer that may have already been freed, leading to a potential
use-after-free.

Remove the VMA accesses from these functions to avoid the race.

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

diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
index da89b3701f5b..3e21e2dabe82 100644
--- a/drivers/accel/amdxdna/aie2_ctx.c
+++ b/drivers/accel/amdxdna/aie2_ctx.c
@@ -1023,8 +1023,6 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
 	kref_get(&mapp->refcnt);
 	up_write(&xdna->notifier_lock);
 
-	XDNA_DBG(xdna, "populate memory range %lx %lx",
-		 mapp->vma->vm_start, mapp->vma->vm_end);
 	mm = mapp->notifier.mm;
 	if (!mmget_not_zero(mm)) {
 		amdxdna_umap_put(mapp);
diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 63976c3bcbe0..80dd183ecba9 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -271,6 +271,10 @@ static bool amdxdna_hmm_invalidate(struct mmu_interval_notifier *mni,
 	if (range->event == MMU_NOTIFY_UNMAP) {
 		down_write(&xdna->notifier_lock);
 		if (!mapp->unmapped) {
+			if (is_import_bo(abo) && mapp->vma->vm_file &&
+			    mapp->vma->vm_file->f_mapping)
+				mapping_clear_unevictable(mapp->vma->vm_file->f_mapping);
+
 			queue_work(xdna->notifier_wq, &mapp->hmm_unreg_work);
 			mapp->unmapped = true;
 		}
@@ -308,12 +312,9 @@ static void amdxdna_umap_release(struct kref *ref)
 {
 	struct amdxdna_umap *mapp = container_of(ref, struct amdxdna_umap, refcnt);
 	struct amdxdna_gem_obj *abo = mapp->abo;
-	struct vm_area_struct *vma = mapp->vma;
 	struct amdxdna_dev *xdna;
 
 	mmu_interval_notifier_remove(&mapp->notifier);
-	if (is_import_bo(abo) && vma->vm_file && vma->vm_file->f_mapping)
-		mapping_clear_unevictable(vma->vm_file->f_mapping);
 
 	xdna = to_xdna_dev(to_gobj(mapp->abo)->dev);
 	down_write(&xdna->notifier_lock);
-- 
2.34.1

Re: [PATCH V1] accel/amdxdna: Fix VMA access race

[sashiko-bot]

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

New issues:
- [High] Use-after-free on `mapp->vma` during MMU notifier invalidation.
- [High] Memory flag `AS_UNEVICTABLE` is permanently leaked on mmap error paths.

Pre-existing issues:
- [High] Pre-existing use-after-free on `mapp->vma` in debug logging.
- [High] Incorrect management of `AS_UNEVICTABLE` on shared file mappings and swapped `vma->vm_file`.
--

commit 7b0235d032796ef659fe1d987ecb0649b81f544e
Author: Lizhi Hou <lizhi.hou@amd.com>

accel/amdxdna: Fix VMA access race

aie2_populate_range() and amdxdna_umap_release() access a saved VMA
pointer that may have already been freed, leading to a potential
use-after-free.

Remove the VMA accesses from these functions to avoid the race.

Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>

> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 63976c3bcbe0f..80dd183ecba92 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -271,6 +271,10 @@ static bool amdxdna_hmm_invalidate(struct mmu_interval_notifier *mni,
>  	if (range->event == MMU_NOTIFY_UNMAP) {
>  		down_write(&xdna->notifier_lock);
>  		if (!mapp->unmapped) {
> +			if (is_import_bo(abo) && mapp->vma->vm_file &&
> +			    mapp->vma->vm_file->f_mapping)
> +				mapping_clear_unevictable(mapp->vma->vm_file->f_mapping);

[Severity: High]
Does this code still have a use-after-free on mapp->vma?

The mmu_interval_notifier tracks virtual address ranges, not VMA lifetimes.
If a VMA is split, merged, or partially unmapped, the original
vm_area_struct can be freed without immediately invalidating the interval
notifier. When a subsequent munmap triggers MMU_NOTIFY_UNMAP, mapp->vma
could be a dangling pointer.


[Severity: High]
Can this code permanently leak the AS_UNEVICTABLE flag on mmap error paths?

If amdxdna_insert_pages() fails during mmap, the error path calls
amdxdna_hmm_unregister(), which sets mapp->unmapped = true but does not
clear the flag. Consequently, the flag might never be cleared because this
invalidation handler skips the clearing when unmapped is true.


[Severity: High]
This isn't a bug introduced by this patch, but does this clear the wrong
file's mapping?

For imported dma-buf BOs, amdxdna_hmm_register() sets AS_UNEVICTABLE on
vma->vm_file->f_mapping. At this time, vma->vm_file is the XDNA device file.

Subsequently, amdxdna_insert_pages() calls dma_buf_mmap(), which swaps
vma->vm_file to the dma-buf file.

When the VMA is later unmapped, mapping_clear_unevictable() is called on the
new vma->vm_file (the dma-buf). This leaves the XDNA device file permanently
locked as unevictable, while the dma-buf file incorrectly loses its
unevictable status for all other clients.


[Severity: High]
This is a pre-existing issue, but does amdxdna_hmm_invalidate() have another
potential use-after-free on the VMA pointer earlier in the function?

Prior to this patch, amdxdna_hmm_invalidate() accesses mapp->vma->vm_start
and mapp->vma->vm_end for an XDNA_DBG log statement:

    XDNA_DBG(xdna, "Invalidating range 0x%lx, 0x%lx, type %d",
             mapp->vma->vm_start, mapp->vma->vm_end, abo->type);

Because VMAs can be freed by mprotect() or vma_merge() without triggering an
MMU notifier, mapp->vma can be a dangling pointer when an invalidation event
later occurs.

> +
>  			queue_work(xdna->notifier_wq, &mapp->hmm_unreg_work);
>  			mapp->unmapped = true;
>  		}

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