Re: [PATCH v2 1/3] drm/xe: Capture devcoredump on TLB invalidation timeout

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

Pre-existing issues:
- [High] The asynchronous devcoredump capture work is never canceled if the driver unloads before it runs, leading to a Use-After-Free of `xe_device` and execution of unmapped module text.
--

--- Patch [1]: [PATCH v2 1/3] drm/xe: Capture devcoredump on TLB invalidation timeout ---
commit 32f12415ddea719f0004ff662a031ef828c48ba8
Author: Tales A. Mendonça <[email protected]>

    drm/xe: Capture devcoredump on TLB invalidation timeout

This commit introduces a variant of xe_devcoredump() designed to capture
the GuC log and CT state on GTs for hangs that aren't tied to an exec queue
or job. It then hooks this up to the TLB invalidation timeout path to
diagnose GuC TLB invalidation ack stalls.

Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678

> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index 5f2b90b18f97b..6bee8933f9578 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -323,40 +318,44 @@ static void xe_devcoredump_deferred_snap_work(struct work_struct *work)
>  }
>  
>  static void devcoredump_snapshot(struct xe_devcoredump *coredump,
> +				 struct xe_gt *gt,
>  				 struct xe_exec_queue *q,
>  				 struct xe_sched_job *job)
>  {

[ ... ]

> -	ss->gt = q->gt;
> +	ss->gt = gt;
>  	INIT_WORK(&ss->work, xe_devcoredump_deferred_snap_work);
>  
>  	/* keep going if fw fails as we still want to save the memory and SW data */

[ ... ]

>  	queue_work(system_dfl_wq, &ss->work);

[Severity: High]
This is a pre-existing issue, but is it possible for this work item to run
after the module is unloaded?

If the driver unbinds or the module unloads before this work executes, the
devm action xe_driver_devcoredump_fini() runs and calls dev_coredump_put().
Since the devcoredump device hasn't been created yet (which happens inside
the work via dev_coredumpm_timeout()), that function does nothing.

The xe_device memory is then freed by devres, and the module text may be
unmapped. When system_dfl_wq eventually executes
xe_devcoredump_deferred_snap_work(), could this result in a Use-After-Free
of the xe_device and a jump to unmapped module memory?

>  
>  	dma_fence_end_signalling(cookie);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1