* [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