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

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

[Pengpeng Hou]

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@vger.kernel.org
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- v1: https://lore.kernel.org/r/20260706094059.82323-1-pengpeng@iscas.ac.cn/
- add Fixes and Cc stable tags

Resend note:
- resend with the complete Cc list; no patch changes

 drivers/mtd/parsers/afs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/parsers/afs.c b/drivers/mtd/parsers/afs.c
index 26116694c821..7ab3d50f565e 100644
--- a/drivers/mtd/parsers/afs.c
+++ b/drivers/mtd/parsers/afs.c
@@ -235,6 +235,9 @@ static int afs_parse_v2_partition(struct mtd_info *mtd,
 	pr_debug("Parsing v2 partition @%08x-%08x\n",
 		 off, off + mtd->erasesize);
 
+	if (mtd->erasesize < sizeof(footer))
+		return -EINVAL;
+
 	/* First read the footer */
 	ptr = off + mtd->erasesize - sizeof(footer);
 	ret = mtd_read(mtd, ptr, sizeof(footer), &sz, (u_char *)footer);
@@ -245,6 +248,8 @@ static int afs_parse_v2_partition(struct mtd_info *mtd,
 	}
 	name = (char *) &footer[0];
 	version = footer[9];
+	if (footer[8] > mtd->erasesize - sizeof(footer))
+		return -EINVAL;
 	ptr = off + mtd->erasesize - sizeof(footer) - footer[8];
 
 	pr_debug("found image \"%s\", version %08x, info @%08x\n",
@@ -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;
 	block_start = imginfo[20];
 	block_end = imginfo[21];
 
-- 
2.53.0


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

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

[Pengpeng Hou]

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@vger.kernel.org
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- v1: https://lore.kernel.org/r/20260706094059.82323-1-pengpeng@iscas.ac.cn/
- add Fixes and Cc stable tags

Resend note:
- resend with the complete Cc list; no patch changes

 drivers/mtd/parsers/afs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/parsers/afs.c b/drivers/mtd/parsers/afs.c
index 26116694c821..7ab3d50f565e 100644
--- a/drivers/mtd/parsers/afs.c
+++ b/drivers/mtd/parsers/afs.c
@@ -235,6 +235,9 @@ static int afs_parse_v2_partition(struct mtd_info *mtd,
 	pr_debug("Parsing v2 partition @%08x-%08x\n",
 		 off, off + mtd->erasesize);
 
+	if (mtd->erasesize < sizeof(footer))
+		return -EINVAL;
+
 	/* First read the footer */
 	ptr = off + mtd->erasesize - sizeof(footer);
 	ret = mtd_read(mtd, ptr, sizeof(footer), &sz, (u_char *)footer);
@@ -245,6 +248,8 @@ static int afs_parse_v2_partition(struct mtd_info *mtd,
 	}
 	name = (char *) &footer[0];
 	version = footer[9];
+	if (footer[8] > mtd->erasesize - sizeof(footer))
+		return -EINVAL;
 	ptr = off + mtd->erasesize - sizeof(footer) - footer[8];
 
 	pr_debug("found image \"%s\", version %08x, info @%08x\n",
@@ -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;
 	block_start = imginfo[20];
 	block_end = imginfo[21];
 
-- 
2.53.0

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@iscas.ac.cn> 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@vger.kernel.org

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@iscas.ac.cn>

> +       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@kernel.org>

Yours,
Linus Walleij

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

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@iscas.ac.cn> 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@vger.kernel.org

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@iscas.ac.cn>

> +       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@kernel.org>

Yours,
Linus Walleij

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

[Miquel Raynal]

Hi Linus,

> 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.

Thanks for the detailed analysis, I also feel like many of these patches
do not fix actual regression, but I am inclined to take them as long as
they do not darken too much the code. In this case, except the very last
change, I am hesitating.

Shall Pengeng extract the last bit (which is useful) and drop the rest?

Thanks,
Miquèl

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

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

[Miquel Raynal]

Hi Linus,

> 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.

Thanks for the detailed analysis, I also feel like many of these patches
do not fix actual regression, but I am inclined to take them as long as
they do not darken too much the code. In this case, except the very last
change, I am hesitating.

Shall Pengeng extract the last bit (which is useful) and drop the rest?

Thanks,
Miquèl