public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
@ 2026-04-08  8:28 Joonas Lahtinen
  2026-04-08  8:34 ` Joonas Lahtinen
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Joonas Lahtinen @ 2026-04-08  8:28 UTC (permalink / raw)
  To: Intel graphics driver community testing & development
  Cc: Direct Rendering Infrastructure - Development, Joonas Lahtinen,
	Ville Syrjälä, Linus Torvalds

Do not pick 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).

Also avoids returning a VMA without increasing the refcount,
which may potentially lead to UAF.

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>
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] 5+ messages in thread

* Re: [PATCH] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
  2026-04-08  8:28 [PATCH] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF Joonas Lahtinen
@ 2026-04-08  8:34 ` Joonas Lahtinen
  2026-04-08  9:08 ` Simona Vetter
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Joonas Lahtinen @ 2026-04-08  8:34 UTC (permalink / raw)
  To: Intel graphics driver community testing & development
  Cc: Direct Rendering Infrastructure - Development,
	Ville Syrjälä, Linus Torvalds

There will be no CI results as Linus already has a temporary fix in his
tree which got to drm-tip via the various -fixes trees. But its the same
change suggested by Ville so effectively it's been tested already in
every CI run after incorporating -rc7 to -fixes trees this week.

Regards, Joonas

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
  2026-04-08  8:28 [PATCH] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF Joonas Lahtinen
  2026-04-08  8:34 ` Joonas Lahtinen
@ 2026-04-08  9:08 ` Simona Vetter
  2026-04-12  2:54 ` Claude review: " Claude Code Review Bot
  2026-04-12  2:54 ` Claude Code Review Bot
  3 siblings, 0 replies; 5+ messages in thread
From: Simona Vetter @ 2026-04-08  9:08 UTC (permalink / raw)
  To: Joonas Lahtinen
  Cc: Intel graphics driver community testing & development,
	Direct Rendering Infrastructure - Development,
	Ville Syrjälä, Linus Torvalds

On Wed, Apr 08, 2026 at 11:28:59AM +0300, Joonas Lahtinen wrote:
> Do not pick 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).
> 
> Also avoids returning a VMA without increasing the refcount,
> which may potentially lead to UAF.
> 
> 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>
> 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))

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.

With the WARN_ON_ONCE added and the above analysis included this is

Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>

Cheers, Sima

PS: Caught a cold, brain's not entirely working, this might be all very
wrong so please check my history digging.

>  			vma = i915_vma_tryget(vma);
> +		else
> +			vma = NULL;
>  		rcu_read_unlock();
>  		if (likely(vma))
>  			return vma;
> -- 
> 2.53.0
> 

-- 
Simona Vetter
Software Engineer
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Claude review: drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
  2026-04-08  8:28 [PATCH] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF Joonas Lahtinen
  2026-04-08  8:34 ` Joonas Lahtinen
  2026-04-08  9:08 ` Simona Vetter
@ 2026-04-12  2:54 ` Claude Code Review Bot
  2026-04-12  2:54 ` Claude Code Review Bot
  3 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-04-12  2:54 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: 3
Reviewed: 2026-04-12T12:54:46.193333

---

This is a single, well-targeted bug fix for a real correctness and potential use-after-free issue in the i915 GEM execbuffer VMA lookup fast path. The fix is minimal (2 lines), correct, and addresses a clear logic gap. The drm-next tree already carries this fix, confirming it has been accepted.

**Verdict: Good patch, should be applied.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Claude review: drm/i915/gem: Don't use VMA from wrong VM in EXECBUF
  2026-04-08  8:28 [PATCH] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF Joonas Lahtinen
                   ` (2 preceding siblings ...)
  2026-04-12  2:54 ` Claude review: " Claude Code Review Bot
@ 2026-04-12  2:54 ` Claude Code Review Bot
  3 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-04-12  2:54 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**The Bug:**

In `eb_lookup_vma()`, the fast path does a radix tree lookup and then checks if the VMA belongs to the correct VM (ppGTT):

```c
vma = radix_tree_lookup(&eb->gem_context->handles_vma, handle);
if (likely(vma && vma->vm == vm))
    vma = i915_vma_tryget(vma);
// BUG: if the condition is false, vma still holds the stale pointer
rcu_read_unlock();
if (likely(vma))
    return vma;  // returns VMA from wrong VM, without refcount!
```

Without the `else vma = NULL;`, when a VMA is found in the radix tree but belongs to a **different** VM (`vma->vm != vm`), the code falls through with `vma` still pointing to the looked-up (wrong-VM) VMA. The subsequent `if (likely(vma))` check sees a non-NULL pointer and **returns it**.

This causes two distinct problems:

1. **Wrong VM VMA returned**: The caller gets a VMA bound to a different address space than intended. If the same BO is used across EXECBUF calls with alternating VMs on the same context, the wrong VMA could be selected, leading to incorrect GPU address translations.

2. **Missing refcount (potential UAF)**: `i915_vma_tryget()` is only called when the VM matches, so the returned VMA has no reference taken on it. Once the RCU read lock is released, nothing prevents the VMA from being freed, creating a use-after-free condition.

**The Fix:**

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

Setting `vma = NULL` on the else branch ensures the fast path is skipped when the VM doesn't match, correctly falling through to the slow path which calls `i915_vma_instance(obj, vm, NULL)` to find or create the proper VMA for the correct VM, with proper refcounting.

**Assessment**: The fix is correct and minimal. The `else` branch also covers the case where `vma` was NULL from the radix tree lookup (a no-op assignment but harmless). The patch is well-described, references the original report, and has appropriate tags. No concerns.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-12  2:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08  8:28 [PATCH] drm/i915/gem: Don't use VMA from wrong VM in EXECBUF Joonas Lahtinen
2026-04-08  8:34 ` Joonas Lahtinen
2026-04-08  9:08 ` Simona Vetter
2026-04-12  2:54 ` Claude review: " Claude Code Review Bot
2026-04-12  2:54 ` 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