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 <[email protected]> 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-584796cb5...@imgtec.com?part=2
