Re: [PATCH v7 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: CCCB state corruption and GPU UAF on job kick failure
- [Medium] drm/imagination: KCCB slot reservation leak on device loss
--

commit ceda87571c98c414f17508dd17e12fa1ee009e5d
Author: Alessio Belle <alessio.belle@imgtec.com>
Date:   Thu Sep 24 07:29:44 2026 +0000

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

This commit modifies pvr_kccb_send_cmd_reserved_powered() to return an int
instead of void, propagating command submission errors up the stack. These
errors now reach the DRM scheduler's run_job() callback.

> 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]
When the device is lost, does this early return leak the KCCB slot reservation?

Looking at pvr_kccb_send_cmd_powered(), it reserves a slot via
pvr_kccb_reserve_slot_sync() which increments pvr_dev->kccb.reserved_count.
It then calls this function. By returning early here, reserved_count is never
decremented, leaking the slot permanently.

>  
>  	mutex_lock(&pvr_ccb->lock);
>  
> -	if (WARN_ON(!pvr_dev->kccb.reserved_count))
> +	if (WARN_ON(!pvr_dev->kccb.reserved_count)) {
> +		err = -EINVAL;
>  		goto out_unlock;
> +	}

[ ... ]

> @@ -374,8 +392,7 @@ pvr_kccb_send_cmd_powered(struct pvr_device *pvr_dev, struct rogue_fwif_kccb_cmd
>  	if (err)
>  		return err;
>  
> -	pvr_kccb_send_cmd_reserved_powered(pvr_dev, cmd, kccb_slot);
> -	return 0;
> +	return pvr_kccb_send_cmd_reserved_powered(pvr_dev, cmd, kccb_slot);
>  }

[ ... ]

> 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
> @@ -793,15 +793,15 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
>  
>  		/* Submit the fragment job along the geometry job and send a combined kick. */
>  		pvr_queue_submit_job_to_cccb(frag_job);
> -		pvr_cccb_send_kccb_combined_kick(pvr_dev,
> -						 &geom_queue->cccb, &frag_queue->cccb,
> -						 pvr_context_get_fw_addr(geom_job->ctx) +
> -						 geom_queue->ctx_offset,
> -						 pvr_context_get_fw_addr(frag_job->ctx) +
> -						 frag_queue->ctx_offset,
> -						 job->hwrt,
> -						 frag_job->fw_ccb_cmd_type ==
> -						 ROGUE_FWIF_CCB_CMD_TYPE_FRAG_PR);
> +		err = pvr_cccb_send_kccb_combined_kick(pvr_dev,
> +						       &geom_queue->cccb, &frag_queue->cccb,
> +						       pvr_context_get_fw_addr(geom_job->ctx) +
> +						       geom_queue->ctx_offset,
> +						       pvr_context_get_fw_addr(frag_job->ctx) +
> +						       frag_queue->ctx_offset,
> +						       job->hwrt,
> +						       frag_job->fw_ccb_cmd_type ==
> +						       ROGUE_FWIF_CCB_CMD_TYPE_FRAG_PR);
>  
>  		trace_pvr_job_submit_fw(geom_job);
>  		trace_pvr_job_submit_fw(frag_job);
> @@ -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]
Does propagating the error here corrupt the CCCB state and potentially cause a
use-after-free?

Looking at the full context of pvr_queue_run_job(), if the kick fails, the
error is caught and handled in the error path:

	if (WARN_ON(err))
		goto err_release;
	
	...

err_release:
	pvr_kccb_release_slot(pvr_dev);
	return ERR_PTR(err);

By this point, pvr_queue_submit_job_to_cccb() has already modified the CCCB
state, including incrementing queue->in_flight_job_count and writing the
commands to the CCCB. The err_release path aborts the DRM scheduler job without
reverting these changes.

If the job is aborted, its resources can be freed by userspace, but its
commands remain in the CCCB. When the next job is successfully kicked, the GPU
will execute the aborted job's commands, leading to a use-after-free of the
freed GPU buffers. Additionally, queue->in_flight_job_count is left permanently
desynchronized.

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