The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] mtd: parsers: afs: add size check to v2 partition
@ 2026-08-24 13:50 Griffin Kroah-Hartman
  2026-08-25 13:42 ` Miquel Raynal
  0 siblings, 1 reply; 3+ messages in thread
From: Griffin Kroah-Hartman @ 2026-08-24 13:50 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-mtd, linux-kernel, Griffin Kroah-Hartman

Add a size check to the loop in afs_parse_v2_partition(), avoiding
walking out of the imginfo[] array bounds if a malicious packet fakes a
large region count.

Assisted-by: gkh_clanker_t1000
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
 drivers/mtd/parsers/afs.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/parsers/afs.c b/drivers/mtd/parsers/afs.c
index 26116694c821..2c6f8768312c 100644
--- a/drivers/mtd/parsers/afs.c
+++ b/drivers/mtd/parsers/afs.c
@@ -287,12 +287,18 @@ static int afs_parse_v2_partition(struct mtd_info *mtd,
 		 block_start, block_end);
 
 	for (i = 0; i < region_count; i++) {
-		u32 region_load_addr = imginfo[pad + 3 + i*4];
-		u32 region_size = imginfo[pad + 4 + i*4];
-		u32 region_offset = imginfo[pad + 5 + i*4];
+		u32 region_load_addr;
+		u32 region_size;
+		u32 region_offset;
 		u32 region_start;
 		u32 region_end;
 
+		if (pad + 5 + i*4 >= ARRAY_SIZE(imginfo))
+			break;
+		region_load_addr = imginfo[pad + 3 + i*4];
+		region_size = imginfo[pad + 4 + i*4];
+		region_offset = imginfo[pad + 5 + i*4];
+
 		pr_debug("  region %d: address: %08x, size: %08x, "
 			 "offset: %08x\n",
 			 i,

---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260824-mtd_break_looper-429d7cf5a7c0

Best regards,
--  
Griffin Kroah-Hartman <griffin@kroah.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] mtd: parsers: afs: add size check to v2 partition
  2026-08-24 13:50 [PATCH] mtd: parsers: afs: add size check to v2 partition Griffin Kroah-Hartman
@ 2026-08-25 13:42 ` Miquel Raynal
  2026-08-25 14:05   ` Griffin Kroah-Hartman
  0 siblings, 1 reply; 3+ messages in thread
From: Miquel Raynal @ 2026-08-25 13:42 UTC (permalink / raw)
  To: Griffin Kroah-Hartman
  Cc: Richard Weinberger, Vignesh Raghavendra, linux-mtd, linux-kernel

Hello Griffin,

Thanks for the patch!

On 24/08/2026 at 15:50:27 +02, Griffin Kroah-Hartman <griffin@kroah.com> wrote:

> Add a size check to the loop in afs_parse_v2_partition(), avoiding
> walking out of the imginfo[] array bounds if a malicious packet fakes a

"packet" refers to network wording, whereas here, in the MTD world, we
would rather talk about a malicious image.

> large region count.
>
> Assisted-by: gkh_clanker_t1000
> Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>

I guess such a fix would require a Fixes tag, as well as Cc'ing the
stable@vger.kernel.org alias.

> ---
>  drivers/mtd/parsers/afs.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/parsers/afs.c b/drivers/mtd/parsers/afs.c
> index 26116694c821..2c6f8768312c 100644
> --- a/drivers/mtd/parsers/afs.c
> +++ b/drivers/mtd/parsers/afs.c
> @@ -287,12 +287,18 @@ static int afs_parse_v2_partition(struct mtd_info *mtd,
>  		 block_start, block_end);
>  
>  	for (i = 0; i < region_count; i++) {
> -		u32 region_load_addr = imginfo[pad + 3 + i*4];
> -		u32 region_size = imginfo[pad + 4 + i*4];
> -		u32 region_offset = imginfo[pad + 5 + i*4];
> +		u32 region_load_addr;
> +		u32 region_size;
> +		u32 region_offset;
>  		u32 region_start;
>  		u32 region_end;
>  
> +		if (pad + 5 + i*4 >= ARRAY_SIZE(imginfo))
> +			break;
> +		region_load_addr = imginfo[pad + 3 + i*4];
> +		region_size = imginfo[pad + 4 + i*4];
> +		region_offset = imginfo[pad + 5 + i*4];
> +

Looking at the code, I see that just above the loop there is an actual
check bailing out early in case the region count is overly big. In
practice:
- pad can only be 0, 1 or 2
- imginfo array size is 36

The check is:

        if (region_count > (ARRAY_SIZE(imginfo) - pad - 3) / 4)
                return -EINVAL;

And the loop goes at most through:

        pad + 5 + (region_count - 1) * 4

So the only possibilities are:

        pad | max(region_count) | max(index)
        ----+-------------------+-----------
        0   | 8                 | 33
        1   | 8                 | 34
        2   | 7                 | 31

It seems like none of those situations could actually lead to a
reachable/exploitable bug. So while I understand the wish for a more
defensive hardening, the check you add seems redundant. I would
therefore suggest to just keep the driver as-is.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mtd: parsers: afs: add size check to v2 partition
  2026-08-25 13:42 ` Miquel Raynal
@ 2026-08-25 14:05   ` Griffin Kroah-Hartman
  0 siblings, 0 replies; 3+ messages in thread
From: Griffin Kroah-Hartman @ 2026-08-25 14:05 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, linux-mtd, linux-kernel


On 8/25/26 3:42 PM, Miquel Raynal wrote:
> Hello Griffin,
>
> Thanks for the patch!
>
> On 24/08/2026 at 15:50:27 +02, Griffin Kroah-Hartman <griffin@kroah.com> wrote:
>
>> Add a size check to the loop in afs_parse_v2_partition(), avoiding
>> walking out of the imginfo[] array bounds if a malicious packet fakes a
> "packet" refers to network wording, whereas here, in the MTD world, we
> would rather talk about a malicious image.
Good to know! thanks for clarifying.
>> ---
>>   drivers/mtd/parsers/afs.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/parsers/afs.c b/drivers/mtd/parsers/afs.c
>> index 26116694c821..2c6f8768312c 100644
>> --- a/drivers/mtd/parsers/afs.c
>> +++ b/drivers/mtd/parsers/afs.c
>> @@ -287,12 +287,18 @@ static int afs_parse_v2_partition(struct mtd_info *mtd,
>>   		 block_start, block_end);
>>   
>>   	for (i = 0; i < region_count; i++) {
>> -		u32 region_load_addr = imginfo[pad + 3 + i*4];
>> -		u32 region_size = imginfo[pad + 4 + i*4];
>> -		u32 region_offset = imginfo[pad + 5 + i*4];
>> +		u32 region_load_addr;
>> +		u32 region_size;
>> +		u32 region_offset;
>>   		u32 region_start;
>>   		u32 region_end;
>>   
>> +		if (pad + 5 + i*4 >= ARRAY_SIZE(imginfo))
>> +			break;
>> +		region_load_addr = imginfo[pad + 3 + i*4];
>> +		region_size = imginfo[pad + 4 + i*4];
>> +		region_offset = imginfo[pad + 5 + i*4];
>> +
> Looking at the code, I see that just above the loop there is an actual
> check bailing out early in case the region count is overly big. In
> practice:
> - pad can only be 0, 1 or 2
> - imginfo array size is 36
>
> The check is:
>
>          if (region_count > (ARRAY_SIZE(imginfo) - pad - 3) / 4)
>                  return -EINVAL;
Sorry, I was not using linux-next, so I did not see that this was 
already checked.
Thank you for the review!

Griffin

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-25 14:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 13:50 [PATCH] mtd: parsers: afs: add size check to v2 partition Griffin Kroah-Hartman
2026-08-25 13:42 ` Miquel Raynal
2026-08-25 14:05   ` Griffin Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox