Re: [PATCH] drm/imagination: Improve logging of unknown freelist IDs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm: missing rate-limiting on firmware-driven log messages
--

commit 48839b908b6b2c9825c18cddcc77e1ed32786c07
Author: Alessio Belle <alessio.belle@imgtec.com>
drm/imagination: Improve logging of unknown freelist IDs

This commit improves logging by adding warnings when the firmware sends
freelist grow or reconstruction requests with unrecognized IDs. It also
removes a WARN_ON() in the grow request path.

> diff --git a/drivers/gpu/drm/imagination/pvr_free_list.c b/drivers/gpu/drm/imagination/pvr_free_list.c
> index e85cac83834c6..f8a333649933a 100644
> --- a/drivers/gpu/drm/imagination/pvr_free_list.c
> +++ b/drivers/gpu/drm/imagination/pvr_free_list.c
[ ... ]
> @@ -367,8 +368,12 @@ void pvr_free_list_process_grow_req(struct pvr_device *pvr_dev,
>  	u32 grow_pages = 0;
>  
>  	/* If we don't have a freelist registered for this ID, we can't do much. */
> -	if (WARN_ON(!free_list))
> +	if (!free_list) {
> +		drm_warn(from_pvr_device(pvr_dev),
> +			 "Received freelist grow request for unknown ID 0x%x\n",
> +			 req->freelist_id);

[Severity: Medium]
Could this lead to log flooding if the firmware repeatedly sends requests
with an unknown freelist ID?

While the firmware is trusted, it is generally standard practice to use
drm_warn_ratelimited() for messages triggered by hardware or firmware
events to prevent kernel log buffer overflows.

>  		return;
> +	}
>  
>  	/* Since the FW made the request, it has already consumed the ready pages,
>  	 * update the host struct.
[ ... ]
> @@ -564,8 +569,12 @@ pvr_free_list_reconstruct(struct pvr_device *pvr_dev, u32 freelist_id)
>  	struct rogue_fwif_freelist *fw_data;
>  	struct pvr_hwrt_data *hwrt_data;
>  
> -	if (!free_list)
> +	if (!free_list) {
> +		drm_warn(from_pvr_device(pvr_dev),
> +			 "Received freelist reconstruction request for unknown ID 0x%x\n",
> +			 freelist_id);

[Severity: Medium]
Similarly, could this unbounded warning consume CPU cycles and mask other
important errors if the firmware malfunctions or is compromised?

Using a rate limited logging function here might be safer.

>  		return;
> +	}
>  
>  	mutex_lock(&free_list->lock);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006-b4-improve-logging-of-unknown-freelist-ids-v1-1-96ebcb132b80@imgtec.com?part=1