Re: [PATCH v4 7/7] drm/i915/gt: Use signalers_lock to prevent starvation of irq_work.

Hello,

On 8/21/26 04:25, Shuangpeng Bai wrote:
> Hi Maarten,
> 
> While reviewing this series, I noticed an additional request lifetime
> issue in signal_irq_work().
> 
> The existing RCU loop removes rq from ce->signals and may then call
> i915_request_put(rq). If that is the final reference, the request can be
> released and its SLAB_TYPESAFE_BY_RCU slot reused before
> list_for_each_entry_rcu() advances and reads rq->signal_link.next. The
> RCU read-side critical section does not provide a stable reference to the
> same request object in this cache.
> 
> The v4 conversion to signalers_lock/ce->signal_lock and the
> list_first_entry_or_null() loop removes this post-put dereference, so it
> also covers this UAF path.
> 
> This is independent of the PREEMPT_RT irq_work starvation trigger. I can
> reach the sequence in a diagnostic run, although KASAN does not reliably
> report it because i915_request uses a SLAB_TYPESAFE_BY_RCU cache.
> 
> Since this also removes a request lifetime UAF from the common signaling
> path, it may be worth considering the relevant fix for stable kernels once
> the series is accepted.

The original patch for the conversion actually had this bug. There were some
failures reported by CI, pointing to a use-after-free after the changes.

Originally I thought it was due to the conversion itself, that the lifetime
was previously sufficiently by RCU. It makes sense that RCU itself couldn't
properly protect that, and that relying on it was a bug to begin with.

Thanks, I'll send this patch again separately and attach a stable tag!
Since you reported the issue, would you want to put your reviewed by
on the patch as well?

Kind regards,
~Maarten Lankhorst