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 <[email protected]> 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/[email protected]?part=1
