Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/i915: Use-after-free of gem_ctx->client in ctx_needs_wa_22013059131() --
commit 8ca882421f7da98e7f060c85d4cf71902e7198d0 Author: Jia Yao <[email protected]> drm/i915/dg2: Add per-client control for Wa_22013059131 Wa_22013059131 sets FORCE_1_SUB_MESSAGE_PER_FRAGMENT in LSC_CHICKEN_BIT_0 at engine init, but this is known to cause GPU hangs in certain workloads. Add I915_CONTEXT_PARAM_WA_22013059131 so userspace that handles the workaround itself (e.g. by limiting SLM size) can set it to 1 to let the kernel know bit 15 programming is not needed. Link: https://github.com/intel/compute-runtime/pull/919 > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c > b/drivers/gpu/drm/i915/gt/intel_lrc.c > index 147d22907960b..4cb8496290638 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c [ ... ] > @@ -1348,6 +1349,60 @@ gen12_invalidate_state_cache(u32 *cs) > return cs; > } > > +static bool ctx_needs_wa_22013059131(const struct intel_context *ce) > +{ > + struct i915_gem_context *gem_ctx; > + bool needed = false; > + > + /* > + * Kernel-internal contexts (gem_context == NULL) never run the > + * shader workloads that require this workaround. > + * > + * For userspace contexts the choice belongs to the client, so latch > + * the kernel as the owner unless the client has already opted out. > + * This batchbuffer is what programs the register, and it is built > + * only once per context (see lrc_pin()), so from here on the client > + * can no longer opt out and set_client_wa_22013059131() will reject > + * any attempt to. > + */ > + rcu_read_lock(); > + gem_ctx = rcu_dereference(ce->gem_context); > + if (gem_ctx) { > + int owner = I915_WA_22013059131_KERNEL; > + > + if (gem_ctx->client) > + owner = > i915_drm_client_latch_wa_22013059131(gem_ctx->client, owner); [Severity: High] Does this introduce a use-after-free regression for gem_ctx->client? The RCU read lock protects gem_ctx from being freed via kfree_rcu(), but it does not protect gem_ctx->client. During context release, i915_gem_context_release_work() synchronously frees the client before the RCU grace period begins: i915_gem_context_release_work() if (ctx->client) i915_drm_client_put(ctx->client); ... kfree_rcu(ctx, rcu); If a user submits a hanging workload and immediately closes the context, the context close drops the gem_ctx refcount to zero. This triggers the release work and synchronously frees gem_ctx->client. Concurrently, if the reset worker calls guc_reset_state() -> lrc_update_regs() -> ctx_needs_wa_22013059131(), it can attempt to read and modify the already-freed client memory via atomic_cmpxchg(). Would acquiring a strong reference via kref_get_unless_zero(&gem_ctx->ref) be required here to safely access the client? > + > + needed = owner != I915_WA_22013059131_USERSPACE; > + } > + rcu_read_unlock(); > + > + return needed; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
