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 <[email protected]>
---
V3:
  fix sashiko comments: error-path cleanup patch race
V2:
  fix sashiko comments: Use-after-free on `mapp->vma`

 drivers/accel/amdxdna/aie2_ctx.c    |  2 --
 drivers/accel/amdxdna/amdxdna_gem.c | 31 +++++++++++++++++++----------
 drivers/accel/amdxdna/amdxdna_gem.h |  1 -
 3 files changed, 21 insertions(+), 13 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..20ce304b19ef 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -254,7 +254,7 @@ static bool amdxdna_hmm_invalidate(struct 
mmu_interval_notifier *mni,
 
        xdna = to_xdna_dev(to_gobj(abo)->dev);
        XDNA_DBG(xdna, "Invalidating range 0x%lx, 0x%lx, type %d",
-                mapp->vma->vm_start, mapp->vma->vm_end, abo->type);
+                mapp->range.start, mapp->range.end, abo->type);
 
        if (!mmu_notifier_range_blockable(range))
                return false;
@@ -284,15 +284,23 @@ static const struct mmu_interval_notifier_ops 
amdxdna_hmm_ops = {
        .invalidate = amdxdna_hmm_invalidate,
 };
 
+static inline bool compare_range(struct amdxdna_umap *mapp,
+                                struct mm_struct *mm,
+                                unsigned long start, unsigned long end)
+{
+       return (!mapp->unmapped && mapp->notifier.mm == mm &&
+               mapp->range.start == start && mapp->range.end == end);
+}
+
 static void amdxdna_hmm_unregister(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_read(&xdna->notifier_lock);
+       down_write(&xdna->notifier_lock);
        list_for_each_entry(mapp, &abo->mem.umap_list, node) {
-               if (!vma || mapp->vma == vma) {
+               if (!vma || compare_range(mapp, vma->vm_mm, vma->vm_start, 
vma->vm_end)) {
                        if (!mapp->unmapped) {
                                queue_work(xdna->notifier_wq, 
&mapp->hmm_unreg_work);
                                mapp->unmapped = true;
@@ -301,19 +309,16 @@ static void amdxdna_hmm_unregister(struct amdxdna_gem_obj 
*abo,
                                break;
                }
        }
-       up_read(&xdna->notifier_lock);
+       up_write(&xdna->notifier_lock);
 }
 
 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);
@@ -355,6 +360,15 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj 
*abo,
                return 0;
        }
 
+       down_read(&xdna->notifier_lock);
+       list_for_each_entry(mapp, &abo->mem.umap_list, node) {
+               if (compare_range(mapp, current->mm, addr, addr + len)) {
+                       up_read(&xdna->notifier_lock);
+                       return 0;
+               }
+       }
+       up_read(&xdna->notifier_lock);
+
        mapp = kzalloc_obj(*mapp);
        if (!mapp)
                return -ENOMEM;
@@ -380,13 +394,10 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj 
*abo,
        mapp->range.start = vma->vm_start;
        mapp->range.end = vma->vm_end;
        mapp->range.default_flags = HMM_PFN_REQ_FAULT;
-       mapp->vma = vma;
        mapp->abo = abo;
        kref_init(&mapp->refcnt);
 
        INIT_WORK(&mapp->hmm_unreg_work, amdxdna_hmm_unreg_work);
-       if (is_import_bo(abo) && vma->vm_file && vma->vm_file->f_mapping)
-               mapping_set_unevictable(vma->vm_file->f_mapping);
 
        down_write(&xdna->notifier_lock);
        if (list_empty(&abo->mem.umap_list))
diff --git a/drivers/accel/amdxdna/amdxdna_gem.h 
b/drivers/accel/amdxdna/amdxdna_gem.h
index a3e44c7a2395..a35d2f15d32c 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.h
+++ b/drivers/accel/amdxdna/amdxdna_gem.h
@@ -12,7 +12,6 @@
 #include "amdxdna_pci_drv.h"
 
 struct amdxdna_umap {
-       struct vm_area_struct           *vma;
        struct mmu_interval_notifier    notifier;
        struct hmm_range                range;
        struct work_struct              hmm_unreg_work;
-- 
2.34.1

Reply via email to