Re: [PATCH] mtd: parsers: afs: add size check to v2 partition

[Miquel Raynal <miquel.raynal@bootlin.com>]

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/