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/i915/gem: Don't use VMA from wrong VM in EXECBUF
Date: Sun, 12 Apr 2026 12:37:22 +1000
Message-ID: <review-patch1-20260408105932.82810-1-joonas.lahtinen@linux.intel.com>
In-Reply-To: <20260408105932.82810-1-joonas.lahtinen@linux.intel.com>
References: <20260408105932.82810-1-joonas.lahtinen@linux.intel.com> <20260408105932.82810-1-joonas.lahtinen@linux.intel.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:** In `eb_lookup_vma()`, the fast-path lookup does:

```c
rcu_read_lock();
vma =3D radix_tree_lookup(&eb->gem_context->handles_vma, handle);
if (likely(vma && vma->vm =3D=3D vm))
    vma =3D i915_vma_tryget(vma);
rcu_read_unlock();
if (likely(vma))
    return vma;
```

When `radix_tree_lookup` succeeds but `vma->vm !=3D vm`, the `if` branch is=
 not taken and `i915_vma_tryget()` is never called. However, `vma` remains =
non-NULL (still pointing to the result of `radix_tree_lookup`). After `rcu_=
read_unlock()`, the `if (likely(vma))` check passes and returns this VMA =
=E2=80=94 from the wrong VM, without a reference count increment. After `rc=
u_read_unlock()`, the VMA could be freed at any time, making this a potenti=
al UAF.

**The fix** adds `else vma =3D NULL;` to reset `vma` when the VM doesn't ma=
tch:

```c
if (likely(vma && vma->vm =3D=3D vm))
    vma =3D i915_vma_tryget(vma);
else
    vma =3D NULL;
```

This is correct. The `else` branch covers two cases:
1. `vma` was NULL from the radix_tree_lookup (no-op, NULL stays NULL).
2. `vma->vm !=3D vm` (the actual bug =E2=80=94 now properly handled by rese=
tting to NULL).

After this fix, when the VM doesn't match, the code falls through to the sl=
ow path which calls `i915_gem_object_lookup()` and `i915_vma_instance()` to=
 properly create/find the correct VMA for the right VM.

**Minor observation:** Simona's analysis suggests adding `WARN_ON_ONCE(vma-=
>vm !=3D vm)` for paranoia, since the mismatch should be impossible with pr=
oto-contexts. The v2 doesn't include this. This seems like a reasonable cho=
ice for a stable-targeted fix =E2=80=94 a WARN_ON would be noisy on older k=
ernels where the mismatch was legitimately possible. That said, if this is =
only going to drm-next, a `WARN_ON_ONCE` after `rcu_read_unlock()` would be=
 a nice debugging aid for catching any future regressions that re-introduce=
 mutable `gem_ctx->vm`. I'd leave this to the maintainer's preference.

**Commit message quality:** Excellent. The Fixes tag, References link, and =
Simona's detailed analysis provide full historical context and rationale. T=
he message correctly identifies both the wrong-VM and the missing-refcount =
aspects of the bug.

**Verdict:** Reviewed-by worthy. The fix is minimal, correct, and well-docu=
mented.

---
Generated by Claude Code Patch Reviewer