[PATCH] drm/panthor: fix firmware control interface bounds checks

[PATCH] drm/panthor: fix firmware control interface bounds checks

[Osama Abdelkader]

panthor_init_cs_iface() and panthor_init_csg_iface() validate firmware
control interface offsets with 32-bit arithmetic and the size of the host
wrapper structures. The offsets are derived from firmware-provided strides,
so the arithmetic can wrap before the bounds check, and the host wrapper
size is not the size of the firmware control interface being mapped.

Use 64-bit arithmetic for the computed offsets and validate against the
actual firmware control interface structure sizes with subtraction-based
bounds checks. Also validate that the shared section is large enough for
the global control interface before using it.

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 | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index eee2bc7e8541..e2fcbd639c3c 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -897,15 +897,16 @@ static int panthor_init_cs_iface(struct panthor_device *ptdev,
 	struct panthor_fw_csg_iface *csg_iface = panthor_fw_get_csg_iface(ptdev, csg_idx);
 	struct panthor_fw_cs_iface *cs_iface = &ptdev->fw->iface.streams[csg_idx][cs_idx];
 	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
-	u32 iface_offset = CSF_GROUP_CONTROL_OFFSET +
-			   (csg_idx * glb_iface->control->group_stride) +
+	u64 iface_offset = CSF_GROUP_CONTROL_OFFSET +
+			   ((u64)csg_idx * glb_iface->control->group_stride) +
 			   CSF_STREAM_CONTROL_OFFSET +
-			   (cs_idx * csg_iface->control->stream_stride);
+			   ((u64)cs_idx * csg_iface->control->stream_stride);
 	struct panthor_fw_cs_iface *first_cs_iface =
 		panthor_fw_get_cs_iface(ptdev, 0, 0);
 
-	if (iface_offset + sizeof(*cs_iface) >= shared_section_sz)
+	if (iface_offset > shared_section_sz ||
+	    sizeof(*cs_iface->control) > shared_section_sz - iface_offset)
 		return -EINVAL;
 
 	spin_lock_init(&cs_iface->lock);
 	cs_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
@@ -955,11 +956,13 @@ static int panthor_init_csg_iface(struct panthor_device *ptdev,
 	struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
 	struct panthor_fw_csg_iface *csg_iface = &ptdev->fw->iface.groups[csg_idx];
 	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
-	u32 iface_offset = CSF_GROUP_CONTROL_OFFSET + (csg_idx * glb_iface->control->group_stride);
+	u64 iface_offset = CSF_GROUP_CONTROL_OFFSET +
+			   ((u64)csg_idx * glb_iface->control->group_stride);
 	unsigned int i;
 
-	if (iface_offset + sizeof(*csg_iface) >= shared_section_sz)
+	if (iface_offset > shared_section_sz ||
+	    sizeof(*csg_iface->control) > shared_section_sz - iface_offset)
 		return -EINVAL;
 
 	spin_lock_init(&csg_iface->lock);
 	csg_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
@@ -1011,12 +1014,16 @@ static u32 panthor_get_instr_features(struct panthor_device *ptdev)
 static int panthor_fw_init_ifaces(struct panthor_device *ptdev)
 {
 	struct panthor_fw_global_iface *glb_iface = &ptdev->fw->iface.global;
+	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
 	unsigned int i;
 
 	if (!ptdev->fw->shared_section->mem->kmap)
 		return -EINVAL;
 
+	if (sizeof(*glb_iface->control) > shared_section_sz)
+		return -EINVAL;
+
 	spin_lock_init(&glb_iface->lock);
 	glb_iface->control = ptdev->fw->shared_section->mem->kmap;
 
 	if (!glb_iface->control->version) {
-- 
2.43.0

Re: [PATCH] drm/panthor: fix firmware control interface bounds checks

[Liviu Dudau]

On Mon, Jul 20, 2026 at 03:44:35PM +0200, Osama Abdelkader wrote:
> panthor_init_cs_iface() and panthor_init_csg_iface() validate firmware
> control interface offsets with 32-bit arithmetic and the size of the host
> wrapper structures. The offsets are derived from firmware-provided strides,
> so the arithmetic can wrap before the bounds check, and the host wrapper
> size is not the size of the firmware control interface being mapped.

How can the offsets wrap with the firmware-provided strides? It's not like
the firmware provides arbitrarily large strides.

> 
> Use 64-bit arithmetic for the computed offsets and validate against the
> actual firmware control interface structure sizes with subtraction-based
> bounds checks. Also validate that the shared section is large enough for
> the global control interface before using it.
> 
> 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 | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index eee2bc7e8541..e2fcbd639c3c 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -897,15 +897,16 @@ static int panthor_init_cs_iface(struct panthor_device *ptdev,
>  	struct panthor_fw_csg_iface *csg_iface = panthor_fw_get_csg_iface(ptdev, csg_idx);
>  	struct panthor_fw_cs_iface *cs_iface = &ptdev->fw->iface.streams[csg_idx][cs_idx];
>  	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> -	u32 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> -			   (csg_idx * glb_iface->control->group_stride) +
> +	u64 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> +			   ((u64)csg_idx * glb_iface->control->group_stride) +
>  			   CSF_STREAM_CONTROL_OFFSET +
> -			   (cs_idx * csg_iface->control->stream_stride);
> +			   ((u64)cs_idx * csg_iface->control->stream_stride);
>  	struct panthor_fw_cs_iface *first_cs_iface =
>  		panthor_fw_get_cs_iface(ptdev, 0, 0);
>  
> -	if (iface_offset + sizeof(*cs_iface) >= shared_section_sz)
> +	if (iface_offset > shared_section_sz ||
> +	    sizeof(*cs_iface->control) > shared_section_sz - iface_offset)
>  		return -EINVAL;
>  
>  	spin_lock_init(&cs_iface->lock);
>  	cs_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
> @@ -955,11 +956,13 @@ static int panthor_init_csg_iface(struct panthor_device *ptdev,
>  	struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
>  	struct panthor_fw_csg_iface *csg_iface = &ptdev->fw->iface.groups[csg_idx];
>  	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> -	u32 iface_offset = CSF_GROUP_CONTROL_OFFSET + (csg_idx * glb_iface->control->group_stride);
> +	u64 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> +			   ((u64)csg_idx * glb_iface->control->group_stride);
>  	unsigned int i;
>  
> -	if (iface_offset + sizeof(*csg_iface) >= shared_section_sz)
> +	if (iface_offset > shared_section_sz ||
> +	    sizeof(*csg_iface->control) > shared_section_sz - iface_offset)
>  		return -EINVAL;
>  
>  	spin_lock_init(&csg_iface->lock);
>  	csg_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
> @@ -1011,12 +1014,16 @@ static u32 panthor_get_instr_features(struct panthor_device *ptdev)
>  static int panthor_fw_init_ifaces(struct panthor_device *ptdev)
>  {
>  	struct panthor_fw_global_iface *glb_iface = &ptdev->fw->iface.global;
> +	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
>  	unsigned int i;
>  
>  	if (!ptdev->fw->shared_section->mem->kmap)
>  		return -EINVAL;
>  
> +	if (sizeof(*glb_iface->control) > shared_section_sz)
> +		return -EINVAL;
> +
>  	spin_lock_init(&glb_iface->lock);
>  	glb_iface->control = ptdev->fw->shared_section->mem->kmap;
>  
>  	if (!glb_iface->control->version) {
> -- 
> 2.43.0
> 

I'm OK with the general content of the patch, so:

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu


-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH] drm/panthor: fix firmware control interface bounds checks

[Osama Abdelkader]

On Tue, Jul 28, 2026 at 04:12:53PM +0100, Liviu Dudau wrote:
> On Mon, Jul 20, 2026 at 03:44:35PM +0200, Osama Abdelkader wrote:
> > panthor_init_cs_iface() and panthor_init_csg_iface() validate firmware
> > control interface offsets with 32-bit arithmetic and the size of the host
> > wrapper structures. The offsets are derived from firmware-provided strides,
> > so the arithmetic can wrap before the bounds check, and the host wrapper
> > size is not the size of the firmware control interface being mapped.
> 
> How can the offsets wrap with the firmware-provided strides? It's not like
> the firmware provides arbitrarily large strides.
>

Thanks for the review. Yes, It's unlikely to happen but good to have,
these patches actually address issues reported by sashiko while reviewing
the first two patches regarding firmware sections with oversized data and
truncated firmware.

> > 
> > Use 64-bit arithmetic for the computed offsets and validate against the
> > actual firmware control interface structure sizes with subtraction-based
> > bounds checks. Also validate that the shared section is large enough for
> > the global control interface before using it.
> > 
> > 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 | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> > index eee2bc7e8541..e2fcbd639c3c 100644
> > --- a/drivers/gpu/drm/panthor/panthor_fw.c
> > +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> > @@ -897,15 +897,16 @@ static int panthor_init_cs_iface(struct panthor_device *ptdev,
> >  	struct panthor_fw_csg_iface *csg_iface = panthor_fw_get_csg_iface(ptdev, csg_idx);
> >  	struct panthor_fw_cs_iface *cs_iface = &ptdev->fw->iface.streams[csg_idx][cs_idx];
> >  	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> > -	u32 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> > -			   (csg_idx * glb_iface->control->group_stride) +
> > +	u64 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> > +			   ((u64)csg_idx * glb_iface->control->group_stride) +
> >  			   CSF_STREAM_CONTROL_OFFSET +
> > -			   (cs_idx * csg_iface->control->stream_stride);
> > +			   ((u64)cs_idx * csg_iface->control->stream_stride);
> >  	struct panthor_fw_cs_iface *first_cs_iface =
> >  		panthor_fw_get_cs_iface(ptdev, 0, 0);
> >  
> > -	if (iface_offset + sizeof(*cs_iface) >= shared_section_sz)
> > +	if (iface_offset > shared_section_sz ||
> > +	    sizeof(*cs_iface->control) > shared_section_sz - iface_offset)
> >  		return -EINVAL;
> >  
> >  	spin_lock_init(&cs_iface->lock);
> >  	cs_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
> > @@ -955,11 +956,13 @@ static int panthor_init_csg_iface(struct panthor_device *ptdev,
> >  	struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
> >  	struct panthor_fw_csg_iface *csg_iface = &ptdev->fw->iface.groups[csg_idx];
> >  	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> > -	u32 iface_offset = CSF_GROUP_CONTROL_OFFSET + (csg_idx * glb_iface->control->group_stride);
> > +	u64 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> > +			   ((u64)csg_idx * glb_iface->control->group_stride);
> >  	unsigned int i;
> >  
> > -	if (iface_offset + sizeof(*csg_iface) >= shared_section_sz)
> > +	if (iface_offset > shared_section_sz ||
> > +	    sizeof(*csg_iface->control) > shared_section_sz - iface_offset)
> >  		return -EINVAL;
> >  
> >  	spin_lock_init(&csg_iface->lock);
> >  	csg_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
> > @@ -1011,12 +1014,16 @@ static u32 panthor_get_instr_features(struct panthor_device *ptdev)
> >  static int panthor_fw_init_ifaces(struct panthor_device *ptdev)
> >  {
> >  	struct panthor_fw_global_iface *glb_iface = &ptdev->fw->iface.global;
> > +	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> >  	unsigned int i;
> >  
> >  	if (!ptdev->fw->shared_section->mem->kmap)
> >  		return -EINVAL;
> >  
> > +	if (sizeof(*glb_iface->control) > shared_section_sz)
> > +		return -EINVAL;
> > +
> >  	spin_lock_init(&glb_iface->lock);
> >  	glb_iface->control = ptdev->fw->shared_section->mem->kmap;
> >  
> >  	if (!glb_iface->control->version) {
> > -- 
> > 2.43.0
> > 
> 
> I'm OK with the general content of the patch, so:
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> Best regards,
> Liviu
> 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

Thank you.
Best regards,
Osama

Re: [PATCH] drm/panthor: fix firmware control interface bounds checks

[Steven Price]

On 20/07/2026 14:44, Osama Abdelkader wrote:
> panthor_init_cs_iface() and panthor_init_csg_iface() validate firmware
> control interface offsets with 32-bit arithmetic and the size of the host
> wrapper structures. The offsets are derived from firmware-provided strides,
> so the arithmetic can wrap before the bounds check, and the host wrapper
> size is not the size of the firmware control interface being mapped.
> 
> Use 64-bit arithmetic for the computed offsets and validate against the
> actual firmware control interface structure sizes with subtraction-based
> bounds checks. Also validate that the shared section is large enough for
> the global control interface before using it.
> 
> Fixes: 2718d91816ee ("drm/panthor: Add the FW logical block")
> Cc: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>

As Liviu pointed out some of this seems a bit unlikely, but I don't
disagree with any of the changes, so:

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

Thanks,
Steve

> ---
>  drivers/gpu/drm/panthor/panthor_fw.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index eee2bc7e8541..e2fcbd639c3c 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -897,15 +897,16 @@ static int panthor_init_cs_iface(struct panthor_device *ptdev,
>  	struct panthor_fw_csg_iface *csg_iface = panthor_fw_get_csg_iface(ptdev, csg_idx);
>  	struct panthor_fw_cs_iface *cs_iface = &ptdev->fw->iface.streams[csg_idx][cs_idx];
>  	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> -	u32 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> -			   (csg_idx * glb_iface->control->group_stride) +
> +	u64 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> +			   ((u64)csg_idx * glb_iface->control->group_stride) +
>  			   CSF_STREAM_CONTROL_OFFSET +
> -			   (cs_idx * csg_iface->control->stream_stride);
> +			   ((u64)cs_idx * csg_iface->control->stream_stride);
>  	struct panthor_fw_cs_iface *first_cs_iface =
>  		panthor_fw_get_cs_iface(ptdev, 0, 0);
>  
> -	if (iface_offset + sizeof(*cs_iface) >= shared_section_sz)
> +	if (iface_offset > shared_section_sz ||
> +	    sizeof(*cs_iface->control) > shared_section_sz - iface_offset)
>  		return -EINVAL;
>  
>  	spin_lock_init(&cs_iface->lock);
>  	cs_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
> @@ -955,11 +956,13 @@ static int panthor_init_csg_iface(struct panthor_device *ptdev,
>  	struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
>  	struct panthor_fw_csg_iface *csg_iface = &ptdev->fw->iface.groups[csg_idx];
>  	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
> -	u32 iface_offset = CSF_GROUP_CONTROL_OFFSET + (csg_idx * glb_iface->control->group_stride);
> +	u64 iface_offset = CSF_GROUP_CONTROL_OFFSET +
> +			   ((u64)csg_idx * glb_iface->control->group_stride);
>  	unsigned int i;
>  
> -	if (iface_offset + sizeof(*csg_iface) >= shared_section_sz)
> +	if (iface_offset > shared_section_sz ||
> +	    sizeof(*csg_iface->control) > shared_section_sz - iface_offset)
>  		return -EINVAL;
>  
>  	spin_lock_init(&csg_iface->lock);
>  	csg_iface->control = ptdev->fw->shared_section->mem->kmap + iface_offset;
> @@ -1011,12 +1014,16 @@ static u32 panthor_get_instr_features(struct panthor_device *ptdev)
>  static int panthor_fw_init_ifaces(struct panthor_device *ptdev)
>  {
>  	struct panthor_fw_global_iface *glb_iface = &ptdev->fw->iface.global;
> +	u64 shared_section_sz = panthor_kernel_bo_size(ptdev->fw->shared_section->mem);
>  	unsigned int i;
>  
>  	if (!ptdev->fw->shared_section->mem->kmap)
>  		return -EINVAL;
>  
> +	if (sizeof(*glb_iface->control) > shared_section_sz)
> +		return -EINVAL;
> +
>  	spin_lock_init(&glb_iface->lock);
>  	glb_iface->control = ptdev->fw->shared_section->mem->kmap;
>  
>  	if (!glb_iface->control->version) {