Re: [PATCH 2/2] drm/xe: Don't free job in TDR

[Matthew Auld <matthew.auld@intel.com>]

On 03/10/2024 01:16, Matthew Brost wrote:
> Freeing job in TDR is not safe as TDR can pass the run_job thread
> resulting in UAF. It is only safe for free job to naturally be called by
> the scheduler. Rather free job in TDR, add to pending list.

s/Rather free/Rather than free/
?

> 
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2811
> Cc: Matthew Auld <matthew.auld@intel.com>
> Fixes: e275d61c5f3f ("drm/xe/guc: Handle timing out of signaled jobs gracefully")
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>

I think we still have the other issue with fence signalling in run_job.

Reviewed-by: Matthew Auld <matthew.auld@intel.com>

> ---
>   drivers/gpu/drm/xe/xe_guc_submit.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 80062e1d3f66..9ecd1661c1b5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1106,10 +1106,13 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>   
>   	/*
>   	 * TDR has fired before free job worker. Common if exec queue
> -	 * immediately closed after last fence signaled.
> +	 * immediately closed after last fence signaled. Add back to pending
> +	 * list so job can be freed and kick scheduler ensuring free job is not
> +	 * lost.
>   	 */
>   	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &job->fence->flags)) {
> -		guc_exec_queue_free_job(drm_job);
> +		xe_sched_add_pending_job(sched, job);
> +		xe_sched_submission_start(sched);
>   
>   		return DRM_GPU_SCHED_STAT_NOMINAL;
>   	}