mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mtd: inftl: validate MediaHeader partition geometry before allocating tables
@ 2026-09-02 12:40 henrymei
  2026-09-03  8:04 ` Miquel Raynal
  0 siblings, 1 reply; 4+ messages in thread
From: henrymei @ 2026-09-02 12:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: miquel.raynal, richard, vigneshr, linux-kernel, Aohan Mei,
	TencentOS Corvus AI, stable

From: Aohan Mei <henrymei@tencent.com>

find_boot_record() trusts the on-flash INFTL MediaHeader partition
fields without validating their relationship to each other or to the
device geometry.

The sanity check

	if ((ip->lastUnit - ip->firstUnit + 1) < ip->virtualUnits)

is evaluated in unsigned 32-bit arithmetic.  With lastUnit < firstUnit
the subtraction wraps to a huge value and the check passes.  Control
then reaches:

	inftl->nb_boot_blocks = ip->firstUnit;    /* loop bound */
	inftl->nb_blocks      = ip->lastUnit + 1; /* table size */

so PUtable/VUtable are allocated with lastUnit + 1 entries while the
boot-block marking loop writes firstUnit u16 entries:

	for (i = 0; i < inftl->nb_boot_blocks; i++)
		inftl->PUtable[i] = BLOCK_RESERVED;

A crafted MediaHeader (e.g. firstUnit=7000, lastUnit=3) turns a
kmalloc_array(4, 2) 8-byte allocation into a ~14 KB out-of-bounds
write:

	BUG: KASAN: slab-out-of-bounds in find_boot_record
	Write of size 2 ... 0 bytes to the right of allocated 8-byte region

The MediaHeader unit itself is also marked through PUtable[block]
without verifying that block lies within the described extent.

Reject inconsistent partition geometry (lastUnit < firstUnit, or
lastUnit beyond the device) as each partition entry is scanned, and
reject a boot record that lies outside the extent of the selected
partition.  The existing virtualUnits check then subtracts values
that can no longer underflow.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
---
 drivers/mtd/inftlmount.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/mtd/inftlmount.c b/drivers/mtd/inftlmount.c
index 87e246a6f488..b0f01db95280 100644
--- a/drivers/mtd/inftlmount.c
+++ b/drivers/mtd/inftlmount.c
@@ -192,6 +192,23 @@ static int find_boot_record(struct INFTLrecord *inftl)
 				 ip->lastUnit, ip->flags,
 				 ip->spareUnits);
 
+			/*
+			 * Reject inconsistent partition geometry before it is
+			 * used: lastUnit < firstUnit would make the
+			 * (lastUnit - firstUnit + 1) check below underflow,
+			 * and lastUnit must stay within the device as it later
+			 * bounds the PUtable/VUtable allocations.
+			 */
+			if (ip->lastUnit < ip->firstUnit ||
+			    ip->lastUnit >= inftl->nb_blocks) {
+				pr_warn("INFTL: Media Header "
+					"Partition %d sanity check failed:\n"
+					"        firstUnit %d lastUnit %d "
+					"(nb_blocks %d)\n",
+					i, ip->firstUnit, ip->lastUnit,
+					inftl->nb_blocks);
+				return -1;
+			}
 			if (ip->Reserved0 != ip->firstUnit) {
 				struct erase_info *instr = &inftl->instr;
 
@@ -233,6 +250,18 @@ static int find_boot_record(struct INFTLrecord *inftl)
 			return -1;
 		}
 
+		/*
+		 * The boot record unit must lie within the described
+		 * extent; it is later marked through PUtable[block].
+		 */
+		if (block > ip->lastUnit) {
+			pr_warn("INFTL: Media Header "
+				"Partition %d sanity check failed:\n"
+				"        boot record unit %d beyond "
+				"lastUnit %d\n",
+				i, block, ip->lastUnit);
+			return -1;
+		}
 		inftl->nb_boot_blocks = ip->firstUnit;
 		inftl->numvunits = ip->virtualUnits;
 		if (inftl->numvunits > (inftl->nb_blocks -
-- 
2.43.7


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

* Re: [PATCH] mtd: inftl: validate MediaHeader partition geometry before allocating tables
  2026-09-02 12:40 [PATCH] mtd: inftl: validate MediaHeader partition geometry before allocating tables henrymei
@ 2026-09-03  8:04 ` Miquel Raynal
  2026-09-03  9:43   ` 林佳鹏
  0 siblings, 1 reply; 4+ messages in thread
From: Miquel Raynal @ 2026-09-03  8:04 UTC (permalink / raw)
  To: henrymei
  Cc: linux-mtd, richard, vigneshr, linux-kernel, Aohan Mei,
	TencentOS Corvus AI, stable

Hi Henry,

On 02/09/2026 at 20:40:19 +08, henrymei <ljp1205831794@gmail.com> wrote:

> From: Aohan Mei <henrymei@tencent.com>
>
> find_boot_record() trusts the on-flash INFTL MediaHeader partition
> fields without validating their relationship to each other or to the
> device geometry.
>
> The sanity check
>
> 	if ((ip->lastUnit - ip->firstUnit + 1) < ip->virtualUnits)
>
> is evaluated in unsigned 32-bit arithmetic.  With lastUnit < firstUnit

lastUnit < firstUnit? Did you even read that sentence?

Sorry but, no.

Thanks,
Miquèl

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

* Re: [PATCH] mtd: inftl: validate MediaHeader partition geometry before allocating tables
  2026-09-03  8:04 ` Miquel Raynal
@ 2026-09-03  9:43   ` 林佳鹏
  2026-09-03 10:01     ` Miquel Raynal
  0 siblings, 1 reply; 4+ messages in thread
From: 林佳鹏 @ 2026-09-03  9:43 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: linux-mtd, richard, vigneshr, linux-kernel, Aohan Mei,
	TencentOS Corvus AI, stable

Hi Miquèl,

On 02/09/2026, Miquel Raynal wrote:
> > is evaluated in unsigned 32-bit arithmetic.  With lastUnit < firstUnit
> 
> lastUnit < firstUnit? Did you even read that sentence?

I should have made the arithmetic explicit in the first place.
All three fields are __u32 (struct INFTLPartition), so with the
crafted header used in testing (firstUnit=7000, lastUnit=3,
virtualUnits=100):

    (3 - 7000 + 1) == 4294960300    /* u32 wrap */
    4294960300 < 100 == false    /* sanity check passes */

Execution then continues with nb_boot_blocks=7000 as the loop
bound against a kmalloc_array(lastUnit + 1 = 4, 2) = 8-byte
PUtable, and the boot-block marking loop writes ~14 KB past
the object.  Reproduced on v7.2-rc4 with a RAM-backed fake
DiskOnChip MTD device carrying that header:

    BUG: KASAN: slab-out-of-bounds in find_boot_record
    Write of size 2 ... 0 bytes to the right of allocated 8-byte region

That said, the reachability is admittedly narrow: this is a
mount-time path, so triggering requires root (device
registration) or physical control of the flash contents;
unprivileged users cannot reach it.  The intent is only
hardening of the MediaHeader parser, in the same spirit as
the sanity checks already in find_boot_record().

One fair point about v1: the new check runs on every partition
entry during the scan, while only the selected BDTL entry's
fields are actually used for the allocations.  If entries with
lastUnit < firstUnit can legitimately appear in other slots
on real media, I can respin to validate only the selected
partition (and drop the boot-record-unit check if preferred).

Happy to send a v2 along those lines if you think the
hardening is worthwhile; otherwise I will drop it.  Either
way, thanks for the time.

Thanks,
Aohan


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

* Re: [PATCH] mtd: inftl: validate MediaHeader partition geometry before allocating tables
  2026-09-03  9:43   ` 林佳鹏
@ 2026-09-03 10:01     ` Miquel Raynal
  0 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2026-09-03 10:01 UTC (permalink / raw)
  To: 林佳鹏
  Cc: linux-mtd, richard, vigneshr, linux-kernel, Aohan Mei,
	TencentOS Corvus AI, stable

On 03/09/2026 at 17:43:04 +08, 林佳鹏 <ljp1205831794@gmail.com> wrote:

> Hi Miquèl,
>
> On 02/09/2026, Miquel Raynal wrote:
>> > is evaluated in unsigned 32-bit arithmetic.  With lastUnit < firstUnit
>> 
>> lastUnit < firstUnit? Did you even read that sentence?
>
> I should have made the arithmetic explicit in the first place.
> All three fields are __u32 (struct INFTLPartition), so with the
> crafted header used in testing (firstUnit=7000, lastUnit=3,
> virtualUnits=100):
>
>     (3 - 7000 + 1) == 4294960300    /* u32 wrap */
>     4294960300 < 100 == false    /* sanity check passes */
>
> Execution then continues with nb_boot_blocks=7000 as the loop
> bound against a kmalloc_array(lastUnit + 1 = 4, 2) = 8-byte
> PUtable, and the boot-block marking loop writes ~14 KB past
> the object.  Reproduced on v7.2-rc4 with a RAM-backed fake
> DiskOnChip MTD device carrying that header:
>
>     BUG: KASAN: slab-out-of-bounds in find_boot_record
>     Write of size 2 ... 0 bytes to the right of allocated 8-byte region
>
> That said, the reachability is admittedly narrow: this is a
> mount-time path, so triggering requires root (device
> registration) or physical control of the flash contents;
> unprivileged users cannot reach it.  The intent is only
> hardening of the MediaHeader parser, in the same spirit as
> the sanity checks already in find_boot_record().
>
> One fair point about v1: the new check runs on every partition
> entry during the scan, while only the selected BDTL entry's
> fields are actually used for the allocations.  If entries with
> lastUnit < firstUnit can legitimately appear in other slots
> on real media, I can respin to validate only the selected
> partition (and drop the boot-record-unit check if preferred).
>
> Happy to send a v2 along those lines if you think the
> hardening is worthwhile; otherwise I will drop it.  Either
> way, thanks for the time.

Since this is a root-only exploit, I don't see the point in going
further. These "hardening" steps just darken the code from my opinion.

Thanks,
Miquèl

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

end of thread, other threads:[~2026-09-03 10:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-02 12:40 [PATCH] mtd: inftl: validate MediaHeader partition geometry before allocating tables henrymei
2026-09-03  8:04 ` Miquel Raynal
2026-09-03  9:43   ` 林佳鹏
2026-09-03 10:01     ` Miquel Raynal

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

all inboxes | Powered by JetHome®