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

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-ffd55254d6d2@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-35355fad50ae@imgtec.com?part=1