From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/panthor: Avoid potential UAF due to memory reclaim
Date: Sun, 12 Apr 2026 09:28:38 +1000
Message-ID: <review-patch1-20260410195050.687201-1-akash.goel@arm.com>
In-Reply-To: <20260410195050.687201-1-akash.goel@arm.com>
References: <20260410195050.687201-1-akash.goel@arm.com> <20260410195050.687201-1-akash.goel@arm.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

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 =E2=80=94 it skipped not just the `drm_gpuvm_bo_evict=
()` call, but the entire VA walk below it. After the fix, only the `drm_gpu=
vm_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_l=
ist_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 =E2=80=94 callin=
g `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 =E2=80=94 the code behavior is corr=
ect either way.

**Locking:** The `mutex_trylock(&vm->op_lock)` path and the `drm_gpuvm_bo_f=
or_each_va` loop are unchanged and remain correct relative to the pre-exist=
ing code.

**Fixes tag and attribution look correct.**

---
Generated by Claude Code Patch Reviewer