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: [email protected] # v5.10+
Signed-off-by: Shuangpeng Bai <[email protected]>
---
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
