[PATCH] drm/panthor: validate firmware interface structure sizes

[PATCH] drm/panthor: validate firmware interface structure sizes

[Osama Abdelkader]

iface_fw_to_cpu_addr() only checks that the firmware-provided MCU virtual
address points inside the shared section. The returned pointer is later
used as a full firmware interface structure, so accepting an address near
the end of the shared section can still lead to out-of-bounds accesses.

Pass the expected object size to iface_fw_to_cpu_addr() and reject ranges
that do not fit entirely in the shared section.

Fixes: 2718d91816ee ("drm/panthor: Add the FW logical block")
Cc: stable@vger.kernel.org
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 drivers/gpu/drm/panthor/panthor_fw.c | 30 ++++++++++++++++++---------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 9d0e1fafdce2..eee2bc7e8541 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -870,18 +870,24 @@ static int panthor_fw_load(struct panthor_device *ptdev)
  * iface_fw_to_cpu_addr() - Turn an MCU address into a CPU address
  * @ptdev: Device.
  * @mcu_va: MCU address.
+ * @size: Size of the object pointed to by @mcu_va.
  *
- * Return: NULL if the address is not part of the shared section, non-NULL otherwise.
+ * Return: NULL if the object is not part of the shared section, non-NULL otherwise.
  */
-static void *iface_fw_to_cpu_addr(struct panthor_device *ptdev, u32 mcu_va)
+static void *iface_fw_to_cpu_addr(struct panthor_device *ptdev, u32 mcu_va, size_t size)
 {
 	u64 shared_mem_start = panthor_kernel_bo_gpuva(ptdev->fw->shared_section->mem);
-	u64 shared_mem_end = shared_mem_start +
-			     panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
-	if (mcu_va < shared_mem_start || mcu_va >= shared_mem_end)
+	size_t shared_mem_size = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
+	u64 offset;
+
+	if (mcu_va < shared_mem_start)
+		return NULL;
+
+	offset = mcu_va - shared_mem_start;
+	if (offset > shared_mem_size || size > shared_mem_size - offset)
 		return NULL;
 
-	return ptdev->fw->shared_section->mem->kmap + (mcu_va - shared_mem_start);
+	return ptdev->fw->shared_section->mem->kmap + offset;
 }
 
 static int panthor_init_cs_iface(struct panthor_device *ptdev,
@@ -906,8 +912,10 @@ static int panthor_init_cs_iface(struct panthor_device *ptdev,
 
 	spin_lock_init(&cs_iface->lock);
 	cs_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
-	cs_iface->input = iface_fw_to_cpu_addr(ptdev, cs_iface->control->input_va);
-	cs_iface->output = iface_fw_to_cpu_addr(ptdev, cs_iface->control->output_va);
+	cs_iface->input = iface_fw_to_cpu_addr(ptdev, cs_iface->control->input_va,
+					       sizeof(*cs_iface->input));
+	cs_iface->output = iface_fw_to_cpu_addr(ptdev, cs_iface->control->output_va,
+						sizeof(*cs_iface->output));
 
 	if (!cs_iface->input || !cs_iface->output) {
 		drm_err(&ptdev->base, "Invalid stream control interface input/output VA");
@@ -957,8 +965,10 @@ static int panthor_init_csg_iface(struct panthor_device *ptdev,
 
 	spin_lock_init(&csg_iface->lock);
 	csg_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
-	csg_iface->input = iface_fw_to_cpu_addr(ptdev, csg_iface->control->input_va);
-	csg_iface->output = iface_fw_to_cpu_addr(ptdev, csg_iface->control->output_va);
+	csg_iface->input = iface_fw_to_cpu_addr(ptdev, csg_iface->control->input_va,
+						sizeof(*csg_iface->input));
+	csg_iface->output = iface_fw_to_cpu_addr(ptdev, csg_iface->control->output_va,
+						 sizeof(*csg_iface->output));
 
 	if (csg_iface->control->stream_num < MIN_CS_PER_CSG ||
 	    csg_iface->control->stream_num > MAX_CS_PER_CSG)
@@ -1015,8 +1025,10 @@ static int panthor_fw_init_ifaces(struct panthor_device *ptdev)
 		return -EINVAL;
 	}
 
-	glb_iface->input = iface_fw_to_cpu_addr(ptdev, glb_iface->control->input_va);
-	glb_iface->output = iface_fw_to_cpu_addr(ptdev, glb_iface->control->output_va);
+	glb_iface->input = iface_fw_to_cpu_addr(ptdev, glb_iface->control->input_va,
+						sizeof(*glb_iface->input));
+	glb_iface->output = iface_fw_to_cpu_addr(ptdev, glb_iface->control->output_va,
+						 sizeof(*glb_iface->output));
 	if (!glb_iface->input || !glb_iface->output) {
 		drm_err(&ptdev->base, "Invalid global control interface input/output VA");
 		return -EINVAL;
-- 
2.43.0

Re: [PATCH] drm/panthor: validate firmware interface structure sizes

[Steven Price]

On 20/07/2026 12:49, Osama Abdelkader wrote:
> iface_fw_to_cpu_addr() only checks that the firmware-provided MCU virtual
> address points inside the shared section. The returned pointer is later
> used as a full firmware interface structure, so accepting an address near
> the end of the shared section can still lead to out-of-bounds accesses.
> 
> Pass the expected object size to iface_fw_to_cpu_addr() and reject ranges
> that do not fit entirely in the shared section.
> 
> Fixes: 2718d91816ee ("drm/panthor: Add the FW logical block")
> Cc: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>

Reviewed-by: Steven Price <steven.price@arm.com>

I'll push this to drm-misc-fixes.

Thanks,
Steve

> ---
>  drivers/gpu/drm/panthor/panthor_fw.c | 30 ++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 9d0e1fafdce2..eee2bc7e8541 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -870,18 +870,24 @@ static int panthor_fw_load(struct panthor_device *ptdev)
>   * iface_fw_to_cpu_addr() - Turn an MCU address into a CPU address
>   * @ptdev: Device.
>   * @mcu_va: MCU address.
> + * @size: Size of the object pointed to by @mcu_va.
>   *
> - * Return: NULL if the address is not part of the shared section, non-NULL otherwise.
> + * Return: NULL if the object is not part of the shared section, non-NULL otherwise.
>   */
> -static void *iface_fw_to_cpu_addr(struct panthor_device *ptdev, u32 mcu_va)
> +static void *iface_fw_to_cpu_addr(struct panthor_device *ptdev, u32 mcu_va, size_t size)
>  {
>  	u64 shared_mem_start = panthor_kernel_bo_gpuva(ptdev->fw->shared_section->mem);
> -	u64 shared_mem_end = shared_mem_start +
> -			     panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> -	if (mcu_va < shared_mem_start || mcu_va >= shared_mem_end)
> +	size_t shared_mem_size = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> +	u64 offset;
> +
> +	if (mcu_va < shared_mem_start)
> +		return NULL;
> +
> +	offset = mcu_va - shared_mem_start;
> +	if (offset > shared_mem_size || size > shared_mem_size - offset)
>  		return NULL;
>  
> -	return ptdev->fw->shared_section->mem->kmap + (mcu_va - shared_mem_start);
> +	return ptdev->fw->shared_section->mem->kmap + offset;
>  }
>  
>  static int panthor_init_cs_iface(struct panthor_device *ptdev,
> @@ -906,8 +912,10 @@ static int panthor_init_cs_iface(struct panthor_device *ptdev,
>  
>  	spin_lock_init(&cs_iface->lock);
>  	cs_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
> -	cs_iface->input = iface_fw_to_cpu_addr(ptdev, cs_iface->control->input_va);
> -	cs_iface->output = iface_fw_to_cpu_addr(ptdev, cs_iface->control->output_va);
> +	cs_iface->input = iface_fw_to_cpu_addr(ptdev, cs_iface->control->input_va,
> +					       sizeof(*cs_iface->input));
> +	cs_iface->output = iface_fw_to_cpu_addr(ptdev, cs_iface->control->output_va,
> +						sizeof(*cs_iface->output));
>  
>  	if (!cs_iface->input || !cs_iface->output) {
>  		drm_err(&ptdev->base, "Invalid stream control interface input/output VA");
> @@ -957,8 +965,10 @@ static int panthor_init_csg_iface(struct panthor_device *ptdev,
>  
>  	spin_lock_init(&csg_iface->lock);
>  	csg_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
> -	csg_iface->input = iface_fw_to_cpu_addr(ptdev, csg_iface->control->input_va);
> -	csg_iface->output = iface_fw_to_cpu_addr(ptdev, csg_iface->control->output_va);
> +	csg_iface->input = iface_fw_to_cpu_addr(ptdev, csg_iface->control->input_va,
> +						sizeof(*csg_iface->input));
> +	csg_iface->output = iface_fw_to_cpu_addr(ptdev, csg_iface->control->output_va,
> +						 sizeof(*csg_iface->output));
>  
>  	if (csg_iface->control->stream_num < MIN_CS_PER_CSG ||
>  	    csg_iface->control->stream_num > MAX_CS_PER_CSG)
> @@ -1015,8 +1025,10 @@ static int panthor_fw_init_ifaces(struct panthor_device *ptdev)
>  		return -EINVAL;
>  	}
>  
> -	glb_iface->input = iface_fw_to_cpu_addr(ptdev, glb_iface->control->input_va);
> -	glb_iface->output = iface_fw_to_cpu_addr(ptdev, glb_iface->control->output_va);
> +	glb_iface->input = iface_fw_to_cpu_addr(ptdev, glb_iface->control->input_va,
> +						sizeof(*glb_iface->input));
> +	glb_iface->output = iface_fw_to_cpu_addr(ptdev, glb_iface->control->output_va,
> +						 sizeof(*glb_iface->output));
>  	if (!glb_iface->input || !glb_iface->output) {
>  		drm_err(&ptdev->base, "Invalid global control interface input/output VA");
>  		return -EINVAL;