* [PATCH v2] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
@ 2026-04-08 10:59 Joonas Lahtinen
2026-04-12 2:37 ` Claude review: " Claude Code Review Bot
2026-04-12 2:37 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Joonas Lahtinen @ 2026-04-08 10:59 UTC (permalink / raw)
To: Intel graphics driver community testing & development
Cc: Direct Rendering Infrastructure - Development, Joonas Lahtinen,
Ville Syrjälä, Linus Torvalds, Simona Vetter,
Tvrtko Ursulin, Andi Shyti, Chris Wilson
Do not pick a VMA with non-matching VM (ppGTT) on quick path
of BO handle lookup for a given EXECBUF call. VMA from wrong VM
could be picked if same BO is repeatedly used in EXECBUF
calls on same context with alternating VMs (ppGTTs). However due
to the introduction of proto-ctx that should not be possible since
d4433c7600f7 ("drm/i915/gem: Use the proto-context to handle
create parameters (v5)").
Also avoids returning a VMA without increasing the refcount,
which may potentially lead to UAF since f7ce8639f6ff ("drm/i915/gem:
Split the context's obj:vma lut into its own mutex") and until
d4433c7600f7 ("drm/i915/gem: Use the proto-context to handle
create parameters (v5)").
Sima's analysis:
This check was added in f7ce8639f6ff ("drm/i915/gem: Split the context's
obj:vma lut into its own mutex") but without any hint in the commit
message as to why. In another hunk of that commit there's a hint though in
__eb_add_lut:
/* user racing with ctx set-vm */
This would mean that this bug was introduced in e0695db7298e ("drm/i915:
Create/destroy VM (ppGTT) for use with contexts"), which allowed to change
the gem_ctx->vm at runtime, opening up the race that was partially fixed
in the earlier referenced commit about a year later.
But it cannot be exploited anymore in anything remotely recent because
with the introduction of proto-contexts we've made gem_ctx->vm invariant
again, exactly to preemptively close all these potential issues.
Specifically d4433c7600f7 ("drm/i915/gem: Use the proto-context to handle
create parameters (v5)") is the vm specific part of the proto-context
work.
Despite that this is impossible to exploit I think it's still good to fix,
but I think for paranoia's sake we should put a WARN_ON_ONCE(vma->vm !=
vm) in there, since this really should be impossible.
I don't think there's a harm in backporting this though, since there's a
2 year window between the introduction of the ctx->vm change and it's
complete fix with the proto-ctx work between 2019 and 2021. It's not
realistic to backport the latter and this here is trivial in case anyone
is foolish enough to run such an old kernel.
Fixes: f7ce8639f6ff ("drm/i915/gem: Split the context's obj:vma lut into its own mutex")
References: https://lore.kernel.org/all/20260324151741.29338-1-sosohero200@gmail.com/
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Linus Torvalds <torvalds@linuxfoundation.org>
Cc: Simona Vetter <simona.vetter@ffwll.ch>
Cc: Tvrtko Ursulin <tursulin@ursulin.net>
Cc: Andi Shyti <andi.shyti@kernel.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index bd608cea396f..7463c3262651 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -897,6 +897,8 @@ static struct i915_vma *eb_lookup_vma(struct i915_execbuffer *eb, u32 handle)
vma = radix_tree_lookup(&eb->gem_context->handles_vma, handle);
if (likely(vma && vma->vm == vm))
vma = i915_vma_tryget(vma);
+ else
+ vma = NULL;
rcu_read_unlock();
if (likely(vma))
return vma;
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Claude review: drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
2026-04-08 10:59 [PATCH v2] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF Joonas Lahtinen
@ 2026-04-12 2:37 ` Claude Code Review Bot
2026-04-12 2:37 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 2:37 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**The bug:** In `eb_lookup_vma()`, the fast-path lookup does:
```c
rcu_read_lock();
vma = radix_tree_lookup(&eb->gem_context->handles_vma, handle);
if (likely(vma && vma->vm == vm))
vma = i915_vma_tryget(vma);
rcu_read_unlock();
if (likely(vma))
return vma;
```
When `radix_tree_lookup` succeeds but `vma->vm != 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 — from the wrong VM, without a reference count increment. After `rcu_read_unlock()`, the VMA could be freed at any time, making this a potential UAF.
**The fix** adds `else vma = NULL;` to reset `vma` when the VM doesn't match:
```c
if (likely(vma && vma->vm == vm))
vma = i915_vma_tryget(vma);
else
vma = 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 != vm` (the actual bug — now properly handled by resetting to NULL).
After this fix, when the VM doesn't match, the code falls through to the slow 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 != vm)` for paranoia, since the mismatch should be impossible with proto-contexts. The v2 doesn't include this. This seems like a reasonable choice for a stable-targeted fix — a WARN_ON would be noisy on older kernels 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. The 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-documented.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread* Claude review: drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
2026-04-08 10:59 [PATCH v2] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF Joonas Lahtinen
2026-04-12 2:37 ` Claude review: " Claude Code Review Bot
@ 2026-04-12 2:37 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 2:37 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
Author: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Patches: 1
Reviewed: 2026-04-12T12:37:22.647331
---
This is a single-patch fix for a bug in the i915 GEM execbuffer VMA lookup fast path. The issue is straightforward: when `radix_tree_lookup` returns a VMA that belongs to a different VM (ppGTT), the code skips the `i915_vma_tryget()` reference acquisition but still falls through with a non-NULL `vma` pointer. This results in returning a VMA from the wrong VM **without** a reference — both a correctness bug and a potential use-after-free.
As the commit message thoroughly explains (with Simona's analysis), this race window was introduced by the runtime VM-change feature in 2019 and was effectively closed in 2021 by making `gem_ctx->vm` invariant via proto-contexts. So the bug is not exploitable on any remotely modern kernel, but the fix is still the right thing to do for code hygiene and for stable backports to older kernels where the window was open.
The fix is minimal, correct, and appropriate for stable backport. **No objections to merging.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-12 2:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08 10:59 [PATCH v2] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF Joonas Lahtinen
2026-04-12 2:37 ` Claude review: " Claude Code Review Bot
2026-04-12 2:37 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox