Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] pvr_kccb_send_cmd_powered() leaks KCCB slot reservations when the underlying command submission fails. --
--- Patch [1]: [PATCH v2] drm/imagination: Propagate all errors from KCCB command submission code --- commit 87e39c90e9f4c3d67e6107ed28de0875ca7379af Author: Alessio Belle <[email protected]> drm/imagination: Propagate all errors from KCCB command submission code This commit updates pvr_kccb_send_cmd_reserved_powered() to return an int instead of void, propagating errors like -EIO and -EINVAL up the call chain. Link: https://patch.msgid.link/20260811-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v1-1-ffd55254d...@imgtec.com > diff --git a/drivers/gpu/drm/imagination/pvr_ccb.c > b/drivers/gpu/drm/imagination/pvr_ccb.c > index 4accf18e23418..8182babd8ad81 100644 > --- a/drivers/gpu/drm/imagination/pvr_ccb.c > +++ b/drivers/gpu/drm/imagination/pvr_ccb.c > @@ -268,19 +273,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; > > 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; > + } > > old_write_offset = READ_ONCE(ctrl->write_offset); > > /* We reserved the slot, we should have one available. */ > - if (WARN_ON(!pvr_ccb_slot_available_locked(pvr_ccb, &new_write_offset))) > + if (WARN_ON(!pvr_ccb_slot_available_locked(pvr_ccb, > &new_write_offset))) { > + err = -EINVAL; > goto out_unlock; > + } > > memcpy(&kccb[old_write_offset], cmd, > sizeof(struct rogue_fwif_kccb_cmd)); [ ... ] > @@ -378,8 +396,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); > } [Severity: Medium] Does this code leak a KCCB slot reservation when the command submission fails? Looking at pvr_kccb_send_cmd_powered(), it calls pvr_kccb_reserve_slot_sync() which increments pvr_dev->kccb.reserved_count on success. If the subsequent call to pvr_kccb_send_cmd_reserved_powered() fails (for example, returning -EIO if pvr_dev->lost is true), it returns early without decrementing the reservation count. Since pvr_kccb_send_cmd_powered() directly returns this error without calling pvr_kccb_release_slot(), this permanently elevates the reserved count, which will eventually trigger a WARN_ON in pvr_kccb_fini() during device teardown. Should pvr_kccb_send_cmd_powered() check the return value and call pvr_kccb_release_slot() on error? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260814-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v2-1-35355fad5...@imgtec.com?part=1
