linux-hardening - Re: [PATCH] mtd: Avoid boot crash in RedBoot partition table parser

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <202602151911.AD092DFFCD@keescook>
Date: Sun, 15 Feb 2026 19:48:08 -0800
From: Kees Cook <kees@...nel.org>
To: Finn Thain <fthain@...ux-m68k.org>
Cc: Miquel Raynal <miquel.raynal@...tlin.com>,
	Richard Weinberger <richard@....at>,
	Vignesh Raghavendra <vigneshr@...com>,
	Miguel Ojeda <ojeda@...nel.org>, stable@...r.kernel.org,
	linux-hardening@...r.kernel.org, linux-mtd@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: Avoid boot crash in RedBoot partition table parser

On Sun, Feb 15, 2026 at 11:21:31AM +1100, Finn Thain wrote:
>     memcmp: detected buffer overflow: 15 byte read of buffer size 14
> [...]
> -		    !memcmp(names, "RedBoot config", 15) ||

The warning is saying that "names" is detected to be 14 bytes in size. It
is allocated here, and nulllen can be ignored (it is fixed size, either 0
or sizeof(nullstring), and skipped over):

        parts = kzalloc(sizeof(*parts) * nrparts + nulllen + namelen, GFP_KERNEL);
	...
        nullname = (char *)&parts[nrparts];
	...
        names = nullname + nulllen;

so "names" is pointing to the final "namelen" many bytes of the
allocation. Calculating "namelen" happens via an earlier for loop:

        buf = vmalloc(master->erasesize);
	...
        ret = mtd_read(master, offset, master->erasesize, &retlen,
                       (void *)buf);
	...
        numslots = (master->erasesize / sizeof(struct fis_image_desc));
	...
        for (i = 0; i < numslots; i++) {
		...
                namelen += strlen(buf[i].name) + 1;

So namelen could be basically any length at all. This fortify warning
looks legit to me -- this code used to be reading beyond the end of
the allocation.

Your patch looks technically correct, but why not just use strcmp? Both
arguments are NUL-terminated. The memcmp() calls were all including the
NUL byte, so they're effectively doing strcmp except that they weren't
stopping at the first NUL byte. So probably just better to do:

diff --git a/drivers/mtd/parsers/redboot.c b/drivers/mtd/parsers/redboot.c
index 3b55b676ca6b..c06ba7a2a34b 100644
--- a/drivers/mtd/parsers/redboot.c
+++ b/drivers/mtd/parsers/redboot.c
@@ -270,9 +270,9 @@ static int parse_redboot_partitions(struct mtd_info *master,
 
 		strcpy(names, fl->img->name);
 #ifdef CONFIG_MTD_REDBOOT_PARTS_READONLY
-		if (!memcmp(names, "RedBoot", 8) ||
-		    !memcmp(names, "RedBoot config", 15) ||
-		    !memcmp(names, "FIS directory", 14)) {
+		if (!strcmp(names, "RedBoot") ||
+		    !strcmp(names, "RedBoot config") ||
+		    !strcmp(names, "FIS directory")) {
 			parts[i].mask_flags = MTD_WRITEABLE;
 		}
 #endif


-- 
Kees Cook