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