Re: [PATCH v11] drm/i915/dg2: Add per-client control for Wa_22013059131

[sashiko-bot@kernel.org]

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 <jia.yao@intel.com>

    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/20260929172320.3481617-1-jia.yao@intel.com?part=1