* [PATCH] Revert "i915: don't use a vma that didn't match the context VM"
@ 2026-05-12 8:58 Joonas Lahtinen
2026-05-12 15:15 ` Linus Torvalds
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Joonas Lahtinen @ 2026-05-12 8:58 UTC (permalink / raw)
To: Intel graphics driver community testing & development
Cc: Direct Rendering Infrastructure - Development, Joonas Lahtinen,
Linus Torvalds, Ville Syrjälä, Tvrtko Ursulin
This reverts commit 5401b9adebc9e5f68df58226f51493ef0e6ceb4d.
Commit is no-op as per discussion in the linked mail thread.
The debated UAF scenario has not been possible in mainline kernel
since 2021.
Link: https://lore.kernel.org/intel-gfx/CAHk-=whfUM8y3PoFfT21+guKWK-mJmAE=8uLzOT+7HGv5NtqSw@mail.gmail.com/
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tursulin@ursulin.net>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 05997e8bbb29..1f303d4eaa4d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -898,8 +898,6 @@ 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 = i915_vma_tryget(vma);
- else
- vma = NULL;
rcu_read_unlock();
if (likely(vma))
return vma;
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Revert "i915: don't use a vma that didn't match the context VM"
2026-05-12 8:58 [PATCH] Revert "i915: don't use a vma that didn't match the context VM" Joonas Lahtinen
@ 2026-05-12 15:15 ` Linus Torvalds
2026-05-16 3:59 ` Claude review: " Claude Code Review Bot
2026-05-16 3:59 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Linus Torvalds @ 2026-05-12 15:15 UTC (permalink / raw)
To: Joonas Lahtinen
Cc: Intel graphics driver community testing & development,
Direct Rendering Infrastructure - Development,
Ville Syrjälä, Tvrtko Ursulin
On Tue, 12 May 2026 at 01:59, Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:
>
> Commit is no-op as per discussion in the linked mail thread.
> The debated UAF scenario has not been possible in mainline kernel
> since 2021.
That was never the issue.
The issue was that the code used to do this:
if (likely(vma && vma->vm == vm))
vma = i915_vma_tryget(vma);
and the vma->vm == vm condition was nonsensical - and if it ever was
false, that code was all horribly buggy.
So the "since 2021" is complete nonsense.
It's only since April this year - commit a13edf9b92fc ("drm/i915/gem:
Drop check for changed VM in EXECBUF") that the "else" side is now
pointless and the revert makes sense.
Stop writing misleading commit logs and excuses for bad code. There is
no "since 2021".
Linus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: Revert "i915: don't use a vma that didn't match the context VM"
2026-05-12 8:58 [PATCH] Revert "i915: don't use a vma that didn't match the context VM" Joonas Lahtinen
2026-05-12 15:15 ` Linus Torvalds
2026-05-16 3:59 ` Claude review: " Claude Code Review Bot
@ 2026-05-16 3:59 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 3:59 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: Revert "i915: don't use a vma that didn't match the context VM"
Author: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Patches: 2
Reviewed: 2026-05-16T13:59:28.286020
---
This is a single-patch revert of commit `5401b9adebc9`, which had added an explicit `else vma = NULL` branch to `eb_lookup_vma()`. The patch is correct: the removed code is indeed a no-op, and the revert is clean and well-justified.
**Verdict: Looks good.**
The `radix_tree_lookup()` function already returns `NULL` when the key is not found, so the `else vma = NULL` assignment is redundant — `vma` is already `NULL` in that branch. The only remaining question is whether the explicit assignment served a defensive/clarity purpose, but as the commit message explains (with a link to the Linus discussion), the original commit was motivated by a UAF concern that hasn't been possible since 2021. Removing dead defensive code that was added under a false premise is reasonable.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: Revert "i915: don't use a vma that didn't match the context VM"
2026-05-12 8:58 [PATCH] Revert "i915: don't use a vma that didn't match the context VM" Joonas Lahtinen
2026-05-12 15:15 ` Linus Torvalds
@ 2026-05-16 3:59 ` Claude Code Review Bot
2026-05-16 3:59 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 3:59 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness: OK**
The patch removes:
```c
else
vma = NULL;
```
from the `eb_lookup_vma()` function in `i915_gem_execbuffer.c`.
This is correct because:
1. `radix_tree_lookup()` returns `void *` and yields `NULL` when the entry is not found (declared in `include/linux/radix-tree.h`). So when the `if (likely(vma))` test falls through, `vma` is already `NULL`.
2. `i915_vma_tryget()` (defined in `i915_vma.h:194`) also returns `NULL` on failure (when `kref_get_unless_zero` fails), so even after the `if (likely(vma))` → `i915_vma_tryget(vma)` path, a failure still leaves `vma` as `NULL`.
3. After the removed `else` branch, the code proceeds to `if (likely(vma)) return vma;` — which correctly handles the `NULL` case regardless.
**Commit message: Good.** References the original commit hash, links the relevant lore discussion with Linus, and explains why the original commit was a no-op. The Cc list is appropriate — includes the original participants in the discussion.
**Style: Fine.** A straightforward revert with `git revert` formatting.
**No concerns.** This is a clean, well-justified removal of dead code.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-16 3:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 8:58 [PATCH] Revert "i915: don't use a vma that didn't match the context VM" Joonas Lahtinen
2026-05-12 15:15 ` Linus Torvalds
2026-05-16 3:59 ` Claude review: " Claude Code Review Bot
2026-05-16 3:59 ` 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