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: Mon, 13 Apr 2026 18:43:07 +1000
Message-ID: <review-patch1-20260413080253.1288157-1-akash.goel@arm.com>
In-Reply-To: <20260413080253.1288157-1-akash.goel@arm.com>
References: <20260413080253.1288157-1-akash.goel@arm.com> <20260413080253.1288157-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

**The Bug:**
When the shrinker evicts a BO, all CPU and GPU mappings are invalidated and=
 `vm_bo->evicted` is set to `true`. If a *new* GPU mapping is subsequently =
created for that BO, the `vm_bo` ends up in a "partially evicted" state =E2=
=80=94 it's back on a reclaim list but `vm_bo->evicted` is still `true`. On=
 the next shrinker invocation, the old code would skip this `vm_bo` entirel=
y:

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

This means the newly created mapping is never torn down, so its backing pag=
es can be reclaimed and reused while still mapped to the GPU =E2=80=94 a cl=
assic use-after-free.

**The Fix:**
The patch replaces the early `continue` with a conditional guard on `drm_gp=
uvm_bo_evict()`, while always walking the VAs to tear down any non-evicted =
mappings:

```c
if (!vm_bo->evicted)
    drm_gpuvm_bo_evict(vm_bo, true);

drm_gpuvm_bo_for_each_va(va, vm_bo) {
```

This is correct =E2=80=94 it ensures every VA on the `vm_bo` is checked for=
 invalidation regardless of the evicted flag.

**Minor observation on the comment:** The comment states that calling `drm_=
gpuvm_bo_evict()` again "would mess up with the internal gpuvm lists." Look=
ing at the underlying `__drm_gpuvm_bo_list_add()` in `drivers/gpu/drm/drm_g=
puvm.c`:

```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 actually protects against double-adds, so a duplic=
ate `drm_gpuvm_bo_evict(vm_bo, true)` call wouldn't corrupt the list =E2=80=
=94 it would just be a redundant no-op. The guard `if (!vm_bo->evicted)` is=
 still good practice (avoids pointless locking and makes the intent clear),=
 but the comment slightly overstates the danger. This is very minor and not=
 worth holding up the patch.

**Verdict:** The fix is correct, minimal, has proper `Fixes:` tag targeting=
 the right commit, and is already reviewed by the relevant maintainer. No i=
ssues blocking merge.

---
Generated by Claude Code Patch Reviewer