Re: [PATCH] drm/xe: Use timer_shutdown_sync() to quiesce the self-rearming irq_fail timer

Re: [PATCH] drm/xe: Use timer_shutdown_sync() to quiesce the self-rearming irq_fail timer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing synchronization between MSI-X interrupt handler and `xe_execlist_port_destroy` leads to UAF of `port->lrc`.
--

--- Patch [1]: [PATCH] drm/xe: Use timer_shutdown_sync() to quiesce the self-rearming irq_fail timer ---
commit 872dd62441863bee2e9f61ac3fe83f0a253f94c8
Author: Fan Wu <fanwu01@zju.edu.cn>

drm/xe: Use timer_shutdown_sync() to quiesce the self-rearming irq_fail timer

This commit updates xe_execlist_port_destroy() to use timer_shutdown_sync()
instead of timer_delete() to safely stop the self-rearming irq_fail timer
and prevent the callback from executing after teardown.

> diff --git a/drivers/gpu/drm/xe/xe_execlist.c b/drivers/gpu/drm/xe/xe_execlist.c
> index 6b86b4f9cc1c5..9ae8f90dbe8f9 100644
> --- a/drivers/gpu/drm/xe/xe_execlist.c
> +++ b/drivers/gpu/drm/xe/xe_execlist.c
> @@ -287,7 +287,7 @@ struct xe_execlist_port *xe_execlist_port_create(struct xe_device *xe,
>  
>  void xe_execlist_port_destroy(struct xe_execlist_port *port)
>  {
> -	timer_delete(&port->irq_fail);
> +	timer_shutdown_sync(&port->irq_fail);
>  
>  	/* Prevent an interrupt while we're destroying */
>  	spin_lock_irq(&gt_to_xe(port->hwe->gt)->irq.lock);

[Severity: High]
This is a pre-existing issue, but can a concurrent MSI-X interrupt still
trigger a use-after-free of port->lrc after this teardown sequence completes?

During teardown, xe_execlist_port_destroy() takes irq.lock, clears
the port->hwe->irq_handler, drops the lock, and then calls
xe_lrc_put(port->lrc).

However, the top-level MSI-X handler (xe_irq_msix_default_hwe_handler())
processes interrupts and dispatches them down to xe_hw_engine_handle_irq(),
which appears to read and execute the handler locklessly:

drivers/gpu/drm/xe/xe_hw_engine.c:xe_hw_engine_handle_irq() {
    ...
    if (hwe->irq_handler)
        hwe->irq_handler(hwe, intr_vec);
    ...
}

Since the interrupt path doesn't acquire irq.lock before checking and
executing the handler, could a concurrent interrupt read the non-NULL
handler, get delayed, and then execute the callback (which accesses
port->lrc) after xe_execlist_port_destroy() has already freed it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717011348.3141550-1-fanwu01@zju.edu.cn?part=1