Re: [PATCH] drm/i915/gem: Fix NULL deref in I915_CONTEXT_PARAM_SSEU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The `user_engines()` function incorrectly applies the SSEU configuration of the first engine (`pe[0]`) to all engines being created, allowing unprivileged userspace to trigger a kernel warning.
- [High] Unhandled error path in `set_proto_ctx_engines()` leaks dynamically allocated `siblings` arrays when an extension chain fails.
- [High] Unconditional intel_runtime_pm_get() in proto_context_set_protected() leaks runtime PM wakerefs if I915_CONTEXT_PARAM_PROTECTED_CONTENT is set multiple times.
--

commit 2f8b7b6ff36e141ab796af7361b851e23a041b50
Author: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Subject: drm/i915/gem: Fix NULL deref in I915_CONTEXT_PARAM_SSEU

This commit fixes a NULL pointer dereference when setting context engine
slot N to I915_ENGINE_CLASS_INVALID or I915_ENGINE_CLASS_INVALID_NONE
and then attempting to apply I915_CONTEXT_PARAM_SSEU to that same slot.

> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index aeafe1742d308..347d1f2c05f50 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -850,7 +850,7 @@ static int set_proto_ctx_sseu(struct drm_i915_file_private *fpriv,
>  		pe = &pc->user_engines[idx];
>  
>  		/* Only render engine supports RPCS configuration. */
> -		if (pe->engine->class != RENDER_CLASS)
> +		if (!pe->engine || pe->engine->class != RENDER_CLASS)
>  			return -EINVAL;
>  
>  		sseu = &pe->sseu;

[Severity: High]
This isn't a bug introduced by this patch, but looking at user_engines()
in the same file, does it incorrectly apply the first engine's SSEU
configuration to all context engines?

If userspace creates a context with a RENDER engine at index 0 and a
non-RENDER engine at index 1, and sets the SSEU configuration for the
RENDER engine:

drivers/gpu/drm/i915/gem/i915_gem_context.c:user_engines()
    ...
    ret = intel_context_set_gem(ce, ctx, pe->sseu);
    ...

It appears user_engines() passes pe->sseu (which evaluates to pe[0].sseu)
instead of pe[n].sseu to intel_context_set_gem().

Since pe[0].sseu.slice_mask is non-zero, will intel_context_set_gem()
evaluate WARN_ON(ce->engine->class != RENDER_CLASS) to true for the
non-RENDER engine and trigger a kernel warning?

[Severity: High]
This is a pre-existing issue, but does the error path in
set_proto_ctx_engines() leak memory?

If userspace submits a I915_CONTEXT_PARAM_ENGINES parameter with an
extension chain containing a valid load balance or parallel submit
extension, memory is dynamically allocated and assigned to
set->engines[idx].siblings.

If a subsequent extension in the chain fails:

drivers/gpu/drm/i915/gem/i915_gem_context.c:set_proto_ctx_engines()
    ...
    if (err) {
            kfree(set.engines);
            return err;
    }
    ...

The error handler frees set.engines, but is it missing a cleanup for the
dynamically allocated siblings arrays inside set.engines[idx]?

[Severity: High]
This is a pre-existing issue, but can proto_context_set_protected() leak
runtime PM wakerefs?

If userspace chains multiple extensions setting
I915_CONTEXT_PARAM_PROTECTED_CONTENT to 1 in a single context creation
call:

drivers/gpu/drm/i915/gem/i915_gem_context.c:proto_context_set_protected()
    ...
    pc->pxp_wakeref = intel_runtime_pm_get(&i915->runtime_pm);
    ...

The function unconditionally calls intel_runtime_pm_get() and overwrites
the pc->pxp_wakeref tracker cookie. When the context is closed,
intel_runtime_pm_put() is only called once.

Could this permanently leak all previous references and prevent the GPU
from entering runtime suspend?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701075555.52142-1-joonas.lahtinen@linux.intel.com?part=1