[PATCH] drm/panthor: Avoid potential UAF due to memory reclaim

[PATCH] drm/panthor: Avoid potential UAF due to memory reclaim

[Akash Goel]

Recent changes to add shrinker support introduced a use after free
vulnerability.
When a BO is evicted from the shrinker callback, all its CPU and GPU
mappings are invalidated. It can happen that another GPU mapping is
created for the BO after the eviction. Because of the new GPU mapping,
BO will be added back to one of the reclaim list but the state of
corresponding vm_bo will not be changed.
If vm_bo remains in evicted state and shrinker callback is invoked
again then the new GPU mapping won't be invalidated. As a result the
backing pages, which were acquired on the creation of new GPU mapping,
can get reclaimed and reused whilst they are still mapped to the GPU.

To prevent the use after free possibility, this commit removes the
evicted check for vm_bo so that all GPU mappings are checked for
invalidation.

Fixes: fb42964e2a76 ("drm/panthor: Add a GEM shrinker")
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Akash Goel <akash.goel@arm.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index fa8b31df85c9..8d0dfa93c45c 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -2350,14 +2350,19 @@ int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo)
 		struct panthor_vm *vm = container_of(vm_bo->vm, struct panthor_vm, base);
 		struct drm_gpuva *va;
 
-		/* Skip already evicted GPU mappings. */
-		if (vm_bo->evicted)
-			continue;
-
 		if (!mutex_trylock(&vm->op_lock))
 			return -EDEADLK;
 
-		drm_gpuvm_bo_evict(vm_bo, true);
+		/* It can be that the vm_bo was already evicted, but a new
+		 * mapping pointing to this BO got created in the meantime,
+		 * turning the vm_bo in partially evicted state. In that case
+		 * we don't call drm_gpuvm_bo_evict() again because this would
+		 * mess up with the internal gpuvm lists, but we do walk the
+		 * VAs on this vm_bo to make sure the non-evicted ones are
+		 * torn down.
+		 */
+		if (!vm_bo->evicted)
+			drm_gpuvm_bo_evict(vm_bo, true);
 		drm_gpuvm_bo_for_each_va(va, vm_bo) {
 			struct panthor_vma *vma = container_of(va, struct panthor_vma, base);
 
-- 
2.25.1

Claude review: drm/panthor: Avoid potential UAF due to memory reclaim

[Claude Code Review Bot]

Overall Series Review

Subject: drm/panthor: Avoid potential UAF due to memory reclaim
Author: Akash Goel <akash.goel@arm.com>
Patches: 1
Reviewed: 2026-04-12T09:28:38.331168

---

This is a single-patch fix for a real use-after-free vulnerability in the panthor GPU driver's shrinker path. The bug was introduced by `fb42964e2a76 ("drm/panthor: Add a GEM shrinker")`. The scenario is:

1. Shrinker evicts a BO, invalidating all GPU mappings and setting `vm_bo->evicted = true`.
2. A new GPU mapping is subsequently created for that BO (adding a VA to the same vm_bo).
3. The BO goes back onto a reclaim list due to the new mapping.
4. Shrinker fires again — but the old code's `if (vm_bo->evicted) continue` skips the entire vm_bo, **including** the VA walk that would tear down the new mapping.
5. Backing pages are freed while still GPU-mapped → UAF.

The fix is correct and well-targeted: it removes the early `continue` and instead only gates the `drm_gpuvm_bo_evict()` call on the evicted flag, while always running the VA invalidation loop.

**Verdict: The patch is correct and fixes a real vulnerability. One minor comment accuracy issue noted below.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/panthor: Avoid potential UAF due to memory reclaim

[Claude Code Review Bot]

Patch Review

**Correctness: Good.** The fix correctly ensures that new VA mappings on an already-evicted vm_bo are still invalidated by the shrinker. The original code:

```c
-		/* Skip already evicted GPU mappings. */
-		if (vm_bo->evicted)
-			continue;
```

...was too aggressive — it skipped not just the `drm_gpuvm_bo_evict()` call, but the entire VA walk below it. After the fix, only the `drm_gpuvm_bo_evict()` call is conditionally skipped:

```c
+		if (!vm_bo->evicted)
+			drm_gpuvm_bo_evict(vm_bo, true);
 		drm_gpuvm_bo_for_each_va(va, vm_bo) {
```

**Minor comment issue.** The new comment states:

```c
+		/* ...we don't call drm_gpuvm_bo_evict() again because this would
+		 * mess up with the internal gpuvm lists...
```

This is technically inaccurate. Looking at the underlying `__drm_gpuvm_bo_list_add()` in `drm_gpuvm.c:871`:

```c
static void
__drm_gpuvm_bo_list_add(struct drm_gpuvm *gpuvm, spinlock_t *lock,
			struct list_head *entry, struct list_head *list)
{
	cond_spin_lock(lock, !!lock);
	if (list_empty(entry))
		list_add_tail(entry, list);
	cond_spin_unlock(lock, !!lock);
}
```

The `list_empty()` guard already prevents double-insertion — calling `drm_gpuvm_bo_evict(vm_bo, true)` a second time would be a harmless no-op (evicted is already `true`, and the list add is guarded). The real reason to skip the call is that it's *redundant*, not that it would corrupt lists. Consider rewording to something like: "we skip the redundant drm_gpuvm_bo_evict() call since the vm_bo is already on the evict list."

This is purely a comment accuracy issue — the code behavior is correct either way.

**Locking:** The `mutex_trylock(&vm->op_lock)` path and the `drm_gpuvm_bo_for_each_va` loop are unchanged and remain correct relative to the pre-existing code.

**Fixes tag and attribution look correct.**

---
Generated by Claude Code Patch Reviewer