[PATCH] drm/i915/gem: Fix request use-after-free in active_engine()

[PATCH] drm/i915/gem: Fix request use-after-free in active_engine()

[Shuangpeng Bai]

active_engine() walks timeline->requests in reverse under RCU and takes a
temporary reference before inspecting each request. However, it drops that
reference in the loop body before list_for_each_entry_reverse() advances
the cursor.

Concurrent retirement can unlink the same request and drop its base
reference while active_engine() holds the temporary reference. The put in
active_engine() may then be final, freeing or recycling the request before
the loop step reads rq->link.prev. SLAB_TYPESAFE_BY_RCU does not defer that
reuse.

Open-code the reverse walk and cache the previous request while the current
request is still referenced. The next request remains protected by
i915_request_get_rcu() and validated against the timeline before use.

An i915 mock selftest forced retirement between the active check and cursor
advance. The vulnerable tree reached the final request release and
kmem_cache_free(), while the fixed tree completed the same ordering without
accessing rq after the put.

Fixes: 3cfea8c97c93 ("drm/i915/gem: Hold request reference for canceling an active context")
Cc: stable@vger.kernel.org # v5.10+
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c58ffa5a8fa6..ff5c892a0176 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1361,7 +1361,7 @@ static bool __cancel_engine(struct intel_engine_cs *engine)
 static struct intel_engine_cs *active_engine(struct intel_context *ce)
 {
 	struct intel_engine_cs *engine = NULL;
-	struct i915_request *rq;
+	struct i915_request *rq, *prev;
 
 	if (intel_context_has_inflight(ce))
 		return intel_context_inflight(ce);
@@ -1375,7 +1375,8 @@ static struct intel_engine_cs *active_engine(struct intel_context *ce)
 	 * (and onto a new timeline->requests list).
 	 */
 	rcu_read_lock();
-	list_for_each_entry_reverse(rq, &ce->timeline->requests, link) {
+	rq = list_last_entry(&ce->timeline->requests, typeof(*rq), link);
+	while (!list_entry_is_head(rq, &ce->timeline->requests, link)) {
 		bool found;
 
 		/* timeline is already completed upto this point? */
@@ -1387,9 +1388,14 @@ static struct intel_engine_cs *active_engine(struct intel_context *ce)
 		if (likely(rcu_access_pointer(rq->timeline) == ce->timeline))
 			found = i915_request_active_engine(rq, &engine);
 
+		/* Cache the cursor before the put, which may release rq. */
+		if (!found)
+			prev = list_prev_entry(rq, link);
 		i915_request_put(rq);
 		if (found)
 			break;
+
+		rq = prev;
 	}
 	rcu_read_unlock();
 

base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4
-- 
2.43.0

✗ LGCI.VerificationFailed: failure for drm/i915/gem: Fix request use-after-free in active_engine()

[Patchwork]

== Series Details ==

Series: drm/i915/gem: Fix request use-after-free in active_engine()
URL   : https://patchwork.freedesktop.org/series/171943/
State : failure

== Summary ==

Series author address 'shuangpeng.kernel@gmail.com' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://gfx-ci.igk.intel.com:8080/job/CI_PW_kernel/183930/ (on built-in)

Re: [PATCH] drm/i915/gem: Fix request use-after-free in active_engine()

[Krzysztof Karas]

Hi Shuangpeng,

On 2026-08-09 at 13:36:46 -0400, Shuangpeng Bai wrote:
> active_engine() walks timeline->requests in reverse under RCU and takes a
> temporary reference before inspecting each request. However, it drops that
> reference in the loop body before list_for_each_entry_reverse() advances
> the cursor.
> 
> Concurrent retirement can unlink the same request and drop its base
> reference while active_engine() holds the temporary reference. The put in
> active_engine() may then be final, freeing or recycling the request before
> the loop step reads rq->link.prev. SLAB_TYPESAFE_BY_RCU does not defer that
> reuse.
> 
> Open-code the reverse walk and cache the previous request while the current
> request is still referenced. The next request remains protected by
> i915_request_get_rcu() and validated against the timeline before use.
> 
> An i915 mock selftest forced retirement between the active check and cursor
> advance. The vulnerable tree reached the final request release and
> kmem_cache_free(), while the fixed tree completed the same ordering without
> accessing rq after the put.
What mock selftest are you referring to?

> 
> Fixes: 3cfea8c97c93 ("drm/i915/gem: Hold request reference for canceling an active context")
> Cc: stable@vger.kernel.org # v5.10+
> Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index c58ffa5a8fa6..ff5c892a0176 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1361,7 +1361,7 @@ static bool __cancel_engine(struct intel_engine_cs *engine)
>  static struct intel_engine_cs *active_engine(struct intel_context *ce)
>  {
>  	struct intel_engine_cs *engine = NULL;
> -	struct i915_request *rq;
> +	struct i915_request *rq, *prev;
>  
>  	if (intel_context_has_inflight(ce))
>  		return intel_context_inflight(ce);
> @@ -1375,7 +1375,8 @@ static struct intel_engine_cs *active_engine(struct intel_context *ce)
>  	 * (and onto a new timeline->requests list).
>  	 */
>  	rcu_read_lock();
> -	list_for_each_entry_reverse(rq, &ce->timeline->requests, link) {
> +	rq = list_last_entry(&ce->timeline->requests, typeof(*rq), link);
> +	while (!list_entry_is_head(rq, &ce->timeline->requests, link)) {
>  		bool found;
>  
>  		/* timeline is already completed upto this point? */
> @@ -1387,9 +1388,14 @@ static struct intel_engine_cs *active_engine(struct intel_context *ce)
>  		if (likely(rcu_access_pointer(rq->timeline) == ce->timeline))
>  			found = i915_request_active_engine(rq, &engine);
>  
> +		/* Cache the cursor before the put, which may release rq. */
> +		if (!found)
You could skip this check here and unconditionally set "prev".
Its value is going to be used only once if found == false anyway.

> +			prev = list_prev_entry(rq, link);
>  		i915_request_put(rq);
>  		if (found)
>  			break;
> +
> +		rq = prev;
>  	}
>  	rcu_read_unlock();
>  
> 
> base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4
> -- 
> 2.43.0
> 

-- 
Best Regards,
Krzysztof