Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Dereferencing `mapp->vma` without holding `mmap_lock` or a VMA lock leads to potential Use-After-Free. -- --- Patch [7]: [PATCH v11 7/8] accel/amdxdna: Use hmm_range_fault_unlocked_timeout() for range population --- commit 49a458a983bff0bc836f15feb7b3aa4763d7eb91 Author: Stanislav Kinsburskii <[email protected]> accel/amdxdna: Use hmm_range_fault_unlocked_timeout() for range population This patch updates aie2_populate_range() to use the new hmm_range_fault_unlocked_timeout() helper. The helper manages the mmap lock and retry sequences internally. The driver now scopes the HMM retry timeout to the helper while maintaining its outer invalidation loop. > diff --git a/drivers/accel/amdxdna/aie2_ctx.c > b/drivers/accel/amdxdna/aie2_ctx.c > index 54486960cbf5e..b5b4ca263002d 100644 > --- a/drivers/accel/amdxdna/aie2_ctx.c > +++ b/drivers/accel/amdxdna/aie2_ctx.c [ ... ] > @@ -1061,24 +1061,9 @@ static int aie2_populate_range(struct amdxdna_gem_obj > *abo) [Severity: High] This isn't a bug introduced by this patch, but I noticed a potential use-after-free with the VMA just before this block in aie2_populate_range(): 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)) { Is it safe to dereference mapp->vma here? Since aie2_populate_range() only holds the xdna->notifier_lock at this point, it doesn't appear to be tied to the VMA's lifetime. If a concurrent munmap from another thread triggers an invalidation and frees the VMA entirely, could the logging statement access a freed VMA structure when reading vm_start and vm_end? > return -EFAULT; > } > > - mapp->range.notifier_seq = mmu_interval_read_begin(&mapp->notifier); > - mmap_read_lock(mm); > - ret = hmm_range_fault(&mapp->range); > - mmap_read_unlock(mm); > - if (ret) { > - if (time_after(jiffies, timeout)) { > - ret = -ETIME; > - goto put_mm; > - } > - > - if (ret == -EBUSY) { > - amdxdna_umap_put(mapp); > - mmput(mm); > - goto again; > - } > - > + ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout); > + if (ret) > goto put_mm; [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
