[PATCH v2] drm/imagination: clamp freelist reconstruction requests

Pengpeng Hou posted 1 patch 4 days, 16 hours ago
drivers/gpu/drm/imagination/pvr_free_list.c |   15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
[PATCH v2] drm/imagination: clamp freelist reconstruction requests
Posted by Pengpeng Hou 4 days, 16 hours ago
The firmware reconstruction count controls accesses to the request's
fixed freelist ID array and the copy into the fixed response array.
Neither access currently bounds the count to those protocol arrays.

Clamp the count to the request capacity, which is shared by the response
layout, and use that count consistently for reconstruction and response
publication. Keep the firmware recovery exchange instead of dropping an
oversized request without a response, as discussed with the firmware
maintainer.

The issue was found by our static-analysis tool.

Fixes: 6eedddab733b ("drm/imagination: Implement free list and HWRT create and destroy ioctls")
Assisted-by: gpt 5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---
Changes since v1:
https://lore.kernel.org/all/20260813152759.35856-1-pengpeng@iscas.ac.cn/
Include drm_print.h for drm_warn_once(), brace the multiline warning
block, and shorten its text as Alessio requested.

 drivers/gpu/drm/imagination/pvr_free_list.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_free_list.c b/drivers/gpu/drm/imagination/pvr_free_list.c
index e85cac83834c..faf5e586d8dc 100644
--- a/drivers/gpu/drm/imagination/pvr_free_list.c
+++ b/drivers/gpu/drm/imagination/pvr_free_list.c
@@ -8,6 +8,7 @@
 #include "pvr_vm.h"
 
 #include <drm/drm_gem.h>
+#include <drm/drm_print.h>
 #include <linux/slab.h>
 #include <linux/xarray.h>
 #include <uapi/drm/pvr_drm.h>
@@ -612,13 +613,21 @@ pvr_free_list_process_reconstruct_req(struct pvr_device *pvr_dev,
 	};
 	struct rogue_fwif_freelists_reconstruction_data *resp =
 		&resp_cmd.cmd_data.free_lists_reconstruction_data;
+	u32 count = min_t(u32, req->freelist_count,
+			  ARRAY_SIZE(req->freelist_ids));
 
-	for (u32 i = 0; i < req->freelist_count; i++)
+	if (count != req->freelist_count) {
+		drm_warn_once(from_pvr_device(pvr_dev),
+			      "Requested reconstruction of %u freelists, limiting to %u\n",
+			      req->freelist_count, count);
+	}
+
+	for (u32 i = 0; i < count; i++)
 		pvr_free_list_reconstruct(pvr_dev, req->freelist_ids[i]);
 
-	resp->freelist_count = req->freelist_count;
+	resp->freelist_count = count;
 	memcpy(resp->freelist_ids, req->freelist_ids,
-	       req->freelist_count * sizeof(resp->freelist_ids[0]));
+	       count * sizeof(resp->freelist_ids[0]));
 
 	WARN_ON(pvr_kccb_send_cmd(pvr_dev, &resp_cmd, NULL));
 }

base-commit: 518e5b794c06c0f0eb40df3e202274a66202c137
-- 
2.50.1 (Apple Git-155)
Re: [PATCH v2] drm/imagination: clamp freelist reconstruction requests
Posted by Brajesh Gupta 11 hours ago
On Sun, 20 Sep 2026 11:43:29 +0800, Pengpeng Hou wrote:
> The firmware reconstruction count controls accesses to the request's
> fixed freelist ID array and the copy into the fixed response array.
> Neither access currently bounds the count to those protocol arrays.
> 
> Clamp the count to the request capacity, which is shared by the response
> layout, and use that count consistently for reconstruction and response
> publication. Keep the firmware recovery exchange instead of dropping an
> oversized request without a response, as discussed with the firmware
> maintainer.
> 
> [...]

Applied to drm-misc-fixes, thanks!

[1/1] drm/imagination: clamp freelist reconstruction requests
      commit: 45585c3aa285854face65293acc95eff73063d6d

Best regards,
-- 
Brajesh Gupta <brajesh.gupta@imgtec.com>
Re: [PATCH v2] drm/imagination: clamp freelist reconstruction requests
Posted by Alessio Belle 12 hours ago
On Sun, 2026-09-20 at 11:43 +0800, Pengpeng Hou wrote:
> The firmware reconstruction count controls accesses to the request's
> fixed freelist ID array and the copy into the fixed response array.
> Neither access currently bounds the count to those protocol arrays.
> 
> Clamp the count to the request capacity, which is shared by the response
> layout, and use that count consistently for reconstruction and response
> publication. Keep the firmware recovery exchange instead of dropping an
> oversized request without a response, as discussed with the firmware
> maintainer.
> 
> The issue was found by our static-analysis tool.
> 
> Fixes: 6eedddab733b ("drm/imagination: Implement free list and HWRT create and destroy ioctls")
> Assisted-by: gpt 5
> Signed-off-by: Pengpeng Hou <hppiscas@163.com>

Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>

Thanks,
Alessio

> ---
> Changes since v1:
> https://lore.kernel.org/all/20260813152759.35856-1-pengpeng@iscas.ac.cn/
> Include drm_print.h for drm_warn_once(), brace the multiline warning
> block, and shorten its text as Alessio requested.
> 
>  drivers/gpu/drm/imagination/pvr_free_list.c |   15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_free_list.c b/drivers/gpu/drm/imagination/pvr_free_list.c
> index e85cac83834c..faf5e586d8dc 100644
> --- a/drivers/gpu/drm/imagination/pvr_free_list.c
> +++ b/drivers/gpu/drm/imagination/pvr_free_list.c
> @@ -8,6 +8,7 @@
>  #include "pvr_vm.h"
>  
>  #include <drm/drm_gem.h>
> +#include <drm/drm_print.h>
>  #include <linux/slab.h>
>  #include <linux/xarray.h>
>  #include <uapi/drm/pvr_drm.h>
> @@ -612,13 +613,21 @@ pvr_free_list_process_reconstruct_req(struct pvr_device *pvr_dev,
>  	};
>  	struct rogue_fwif_freelists_reconstruction_data *resp =
>  		&resp_cmd.cmd_data.free_lists_reconstruction_data;
> +	u32 count = min_t(u32, req->freelist_count,
> +			  ARRAY_SIZE(req->freelist_ids));
>  
> -	for (u32 i = 0; i < req->freelist_count; i++)
> +	if (count != req->freelist_count) {
> +		drm_warn_once(from_pvr_device(pvr_dev),
> +			      "Requested reconstruction of %u freelists, limiting to %u\n",
> +			      req->freelist_count, count);
> +	}
> +
> +	for (u32 i = 0; i < count; i++)
>  		pvr_free_list_reconstruct(pvr_dev, req->freelist_ids[i]);
>  
> -	resp->freelist_count = req->freelist_count;
> +	resp->freelist_count = count;
>  	memcpy(resp->freelist_ids, req->freelist_ids,
> -	       req->freelist_count * sizeof(resp->freelist_ids[0]));
> +	       count * sizeof(resp->freelist_ids[0]));
>  
>  	WARN_ON(pvr_kccb_send_cmd(pvr_dev, &resp_cmd, NULL));
>  }
> 
> base-commit: 518e5b794c06c0f0eb40df3e202274a66202c137