Re: [PATCH v4] drm/i915/gem: Drop check for changed VM in EXECBUF

[Tvrtko Ursulin <tursulin@ursulin.net>]

On 09/04/2026 06:31, Joonas Lahtinen wrote:
> Since the introduction of d4433c7600f7 ("drm/i915/gem: Use the proto-context
> to handle create parameters (v5)") it has not been possible for VM to change
> after context creation so the check will never fail.
> 
> 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.
> 
> v3:
> - Include Sima's analysis and WARN_ON_ONCE
> 
> v4:
> - Focus only on latest mainline codebase
> 
> 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>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index bd608cea396f..621f07a55c38 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -895,7 +895,7 @@ static struct i915_vma *eb_lookup_vma(struct i915_execbuffer *eb, u32 handle)
>   
>   		rcu_read_lock();
>   		vma = radix_tree_lookup(&eb->gem_context->handles_vma, handle);
> -		if (likely(vma && vma->vm == vm))
> +		if (likely(vma))
>   			vma = i915_vma_tryget(vma);
>   		rcu_read_unlock();
>   		if (likely(vma))

Analysis reads correct to me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko