Re: [PATCH v6 2/3] drm/imagination: Propagate all errors from KCCB command submission code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/imagination: use-after-free and state corruption due to missing CCCB rollback on KCCB kick failure
- [Medium] drm/imagination: resource leak of KCCB slot reservations on error
--

commit a2bdc84028e3fc1451749b3764c3318c1a8c6d75
Author: Alessio Belle <alessio.belle@imgtec.com>

drm/imagination: Propagate all errors from KCCB command submission code

This commit updates the KCCB command submission code to propagate errors
back to the caller. The DRM scheduler's run_job callback is updated to
return these errors instead of unconditionally succeeding.

> diff --git a/drivers/gpu/drm/imagination/pvr_ccb.c b/drivers/gpu/drm/imagination/pvr_ccb.c
> --- a/drivers/gpu/drm/imagination/pvr_ccb.c
> +++ b/drivers/gpu/drm/imagination/pvr_ccb.c
[ ... ]
> @@ -264,19 +269,25 @@ pvr_kccb_send_cmd_reserved_powered(struct pvr_device *pvr_dev,
>  	struct rogue_fwif_ccb_ctl *ctrl = pvr_ccb->ctrl;
>  	u32 old_write_offset;
>  	u32 new_write_offset;
> +	int err;
>  
> -	WARN_ON(pvr_dev->lost);
> +	if (pvr_dev->lost)
> +		return -EIO;

[Severity: Medium]
If we return early here, does it leak the KCCB slot reservation?

When this is called from pvr_kccb_send_cmd_powered(), a slot has already 
been reserved by incrementing pvr_dev->kccb.reserved_count. The success 
path decrements this count later in the function, but this early return 
does not, which could lead to slot starvation and eventual -EBUSY errors 
on all future KCCB submissions.

[ ... ]
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c
[ ... ]
> @@ -809,9 +809,10 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
>  		struct pvr_queue *queue = container_of(job->base.sched,
>  						       struct pvr_queue, scheduler);
>  
> -		pvr_cccb_send_kccb_kick(pvr_dev, &queue->cccb,
> -					pvr_context_get_fw_addr(job->ctx) + queue->ctx_offset,
> -					job->hwrt);
> +		err = pvr_cccb_send_kccb_kick(pvr_dev, &queue->cccb,
> +					      pvr_context_get_fw_addr(job->ctx) +
> +					      queue->ctx_offset,
> +					      job->hwrt);
>  
>  		trace_pvr_job_submit_fw(job);
>  	}

[Severity: High]
If pvr_cccb_send_kccb_kick() fails here and pvr_queue_run_job() returns 
ERR_PTR(err), could this lead to a use-after-free and state corruption?

Earlier in pvr_queue_run_job(), pvr_queue_submit_job_to_cccb() is called 
which increments queue->in_flight_job_count and writes the job's commands 
into the firmware's CCCB ring buffer. If the kick fails and the DRM 
scheduler frees this job, the commands remain in the CCCB without being 
rolled back. 

When a subsequent job is successfully submitted, won't the firmware 
process the leftover commands from the freed job, accessing freed resources?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v6-0-d9d5375665ed@imgtec.com?part=2