Re: [PATCH v2 RESEND] mtd: afs: validate v2 image info bounds

[Linus Walleij]

Hi Pengpeng,

thanks for your patch!

On Wed, Jul 8, 2026 at 3:49 AM Pengpeng Hou <pengpeng@xxxxxxxxxxx> wrote:

> The AFS v2 parser uses footer[8] to locate the image information block
> inside the current erase block, then uses the image information
> region_count to walk entries from a fixed local array. The footer offset
> and region count come from flash contents and are not checked against the
> erase block or the local image-info array before use.
>
> Reject v2 entries whose image information offset would underflow the
> erase block calculation, and reject region counts that cannot fit in the
> local image-info array before walking region entries.
>
> Fixes: b7cf5e2830bb ("mtd: afs: add v2 partition parsing")
> Cc: stable@xxxxxxxxxxxxxxx

I don't know if this is stable material. No-one is running into any
regressions, I think this was discovered by code analysis and
is mostly theoretical problems.

> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>

> +       if (mtd->erasesize < sizeof(footer))
> +               return -EINVAL;

You are aware that no system has an erase block size less than
64KB in practice, and sizeof(footer) is 48 bytes?
This is why I say this kind of "bugs" are pretty
theoretical.

>         name = (char *) &footer[0];
>         version = footer[9];
> +       if (footer[8] > mtd->erasesize - sizeof(footer))
> +               return -EINVAL;

The same theoretical thing here. footer[8] is 0..255 but
erasesize is at least 65535 and sizeof(footer) is 48 bytes.

In essence this code translates to:

is something between 0 and 255 more than 65535 (or more) - 48?

The compiler will of course just optimize this out, but I'm just
pointing it out.

I understand this kind of fixes are aesthetically pleasing
but they are not bugs. More like ornaments for coders.

> @@ -278,6 +283,8 @@ static int afs_parse_v2_partition(struct mtd_info *mtd,
>         entrypoint = imginfo[pad];
>         attributes = imginfo[pad+1];
>         region_count = imginfo[pad+2];
> +       if (region_count > (ARRAY_SIZE(imginfo) - pad - 3) / 4)
> +               return -EINVAL;

This is valid sanitization of region_count however!

Acked-by: Linus Walleij <linusw@xxxxxxxxxx>

Yours,
Linus Walleij