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 <[email protected]> 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-96ebcb132...@imgtec.com?part=1
