[PATCH v4 0/6] initramfs: test and improve cpio hex header validation

[PATCH v4 0/6] initramfs: test and improve cpio hex header validation

[David Disseldorp]

Changes v4:
- rework initramfs_test changes to use an inject_ox parameter instead of
  having tests pass through an unvalidated format string.
  + drop Andy's reviewed-by from 1/6 and 2/6 to account for changed test
  + 4/6 minor changes for test context and comment
- add review tags for 3/6, 5/6 and 6/6 following list feedback

----------------------------------------------------------------
v3: 20260323150054.3587083-1-andriy.shevchenko@linux.intel.com

The series that introduced simple_strntoul() had passed into kernel
without proper review and hence reinvented a wheel that's not needed.
Here is the refactoring to show that. It can go via PRINTK or VFS
tree.

I have tested this on x86, but I believe the same result will be
on big-endian CPUs (I deduced that from how strtox() works).

I also run KUnit tests.

Changelog v3:
- rebased on top of latest kernel
- squashed patches proposed by David

v2: 20260121172749.32322-1-ddiss@suse.de
v1: 20260119204151.1447503-1-andriy.shevchenko@linux.intel.com

----------------------------------------------------------------
Andy Shevchenko (4):
      initramfs: Sort headers alphabetically
      initramfs: Refactor to use hex2bin() instead of custom approach
      vsprintf: Revert "add simple_strntoul"
      kstrtox: Drop extern keyword in the simple_strtox() declarations

David Disseldorp (2):
      initramfs_test: add fill_cpio() inject_ox parameter
      initramfs_test: test header fields with 0x hex prefix

 include/linux/kstrtox.h |  9 +++--
 init/initramfs.c        | 68 ++++++++++++++++++++----------------
 init/initramfs_test.c   | 76 ++++++++++++++++++++++++++++++++++-------
 lib/vsprintf.c          |  7 ----
 4 files changed, 105 insertions(+), 55 deletions(-)

[PATCH v4 1/6] initramfs_test: add fill_cpio() inject_ox parameter

[David Disseldorp]

fill_cpio() uses sprintf() to write out the in-memory cpio archive from
an array of struct initramfs_test_cpio. This change allows callers to
modify the cpio sprintf() format string so that future tests can
intentionally corrupt the header with "0x" and "0X" prefixed fields.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 init/initramfs_test.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/init/initramfs_test.c b/init/initramfs_test.c
index 2ce38d9a8fd0f..3ec7591660e03 100644
--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -27,7 +27,17 @@ struct initramfs_test_cpio {
 	char *data;
 };
 
-static size_t fill_cpio(struct initramfs_test_cpio *cs, size_t csz, char *out)
+/* regular newc header format */
+#define CPIO_HDR_FMT "%s%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%s"
+/*
+ * bogus newc header with "0x" prefixes on the uid, gid and namesize values.
+ * parse_header()/simple_str[n]toul() accept this.
+ */
+#define CPIO_HDR_OX_INJECT \
+	"%s%08x%08x0x%06x0X%06x%08x%08x%08x%08x%08x%08x%08x0x%06x%08x%s"
+
+static size_t fill_cpio(struct initramfs_test_cpio *cs, size_t csz,
+			bool inject_ox, char *out)
 {
 	int i;
 	size_t off = 0;
@@ -38,9 +48,8 @@ static size_t fill_cpio(struct initramfs_test_cpio *cs, size_t csz, char *out)
 		size_t thislen;
 
 		/* +1 to account for nulterm */
-		thislen = sprintf(pos, "%s"
-			"%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x"
-			"%s",
+		thislen = sprintf(pos,
+			inject_ox ? CPIO_HDR_OX_INJECT : CPIO_HDR_FMT,
 			c->magic, c->ino, c->mode, c->uid, c->gid, c->nlink,
 			c->mtime, c->filesize, c->devmajor, c->devminor,
 			c->rdevmajor, c->rdevminor, c->namesize, c->csum,
@@ -102,7 +111,7 @@ static void __init initramfs_test_extract(struct kunit *test)
 	/* +3 to cater for any 4-byte end-alignment */
 	cpio_srcbuf = kzalloc(ARRAY_SIZE(c) * (CPIO_HDRLEN + PATH_MAX + 3),
 			      GFP_KERNEL);
-	len = fill_cpio(c, ARRAY_SIZE(c), cpio_srcbuf);
+	len = fill_cpio(c, ARRAY_SIZE(c), false, cpio_srcbuf);
 
 	ktime_get_real_ts64(&ts_before);
 	err = unpack_to_rootfs(cpio_srcbuf, len);
@@ -177,7 +186,7 @@ static void __init initramfs_test_fname_overrun(struct kunit *test)
 	/* limit overrun to avoid crashes / filp_open() ENAMETOOLONG */
 	cpio_srcbuf[CPIO_HDRLEN + strlen(c[0].fname) + 20] = '\0';
 
-	len = fill_cpio(c, ARRAY_SIZE(c), cpio_srcbuf);
+	len = fill_cpio(c, ARRAY_SIZE(c), false, cpio_srcbuf);
 	/* overwrite trailing fname terminator and padding */
 	suffix_off = len - 1;
 	while (cpio_srcbuf[suffix_off] == '\0') {
@@ -219,7 +228,7 @@ static void __init initramfs_test_data(struct kunit *test)
 	cpio_srcbuf = kmalloc(CPIO_HDRLEN + c[0].namesize + c[0].filesize + 6,
 			      GFP_KERNEL);
 
-	len = fill_cpio(c, ARRAY_SIZE(c), cpio_srcbuf);
+	len = fill_cpio(c, ARRAY_SIZE(c), false, cpio_srcbuf);
 
 	err = unpack_to_rootfs(cpio_srcbuf, len);
 	KUNIT_EXPECT_NULL(test, err);
@@ -274,7 +283,7 @@ static void __init initramfs_test_csum(struct kunit *test)
 
 	cpio_srcbuf = kmalloc(8192, GFP_KERNEL);
 
-	len = fill_cpio(c, ARRAY_SIZE(c), cpio_srcbuf);
+	len = fill_cpio(c, ARRAY_SIZE(c), false, cpio_srcbuf);
 
 	err = unpack_to_rootfs(cpio_srcbuf, len);
 	KUNIT_EXPECT_NULL(test, err);
@@ -284,7 +293,7 @@ static void __init initramfs_test_csum(struct kunit *test)
 
 	/* mess up the csum and confirm that unpack fails */
 	c[0].csum--;
-	len = fill_cpio(c, ARRAY_SIZE(c), cpio_srcbuf);
+	len = fill_cpio(c, ARRAY_SIZE(c), false, cpio_srcbuf);
 
 	err = unpack_to_rootfs(cpio_srcbuf, len);
 	KUNIT_EXPECT_NOT_NULL(test, err);
@@ -330,7 +339,7 @@ static void __init initramfs_test_hardlink(struct kunit *test)
 
 	cpio_srcbuf = kmalloc(8192, GFP_KERNEL);
 
-	len = fill_cpio(c, ARRAY_SIZE(c), cpio_srcbuf);
+	len = fill_cpio(c, ARRAY_SIZE(c), false, cpio_srcbuf);
 
 	err = unpack_to_rootfs(cpio_srcbuf, len);
 	KUNIT_EXPECT_NULL(test, err);
@@ -371,7 +380,7 @@ static void __init initramfs_test_many(struct kunit *test)
 		};
 
 		c.namesize = 1 + sprintf(thispath, "initramfs_test_many-%d", i);
-		p += fill_cpio(&c, 1, p);
+		p += fill_cpio(&c, 1, false, p);
 	}
 
 	len = p - cpio_srcbuf;
@@ -425,7 +434,7 @@ static void __init initramfs_test_fname_pad(struct kunit *test)
 	} };
 
 	memcpy(tbufs->padded_fname, "padded_fname", sizeof("padded_fname"));
-	len = fill_cpio(c, ARRAY_SIZE(c), tbufs->cpio_srcbuf);
+	len = fill_cpio(c, ARRAY_SIZE(c), false, tbufs->cpio_srcbuf);
 
 	err = unpack_to_rootfs(tbufs->cpio_srcbuf, len);
 	KUNIT_EXPECT_NULL(test, err);
@@ -481,7 +490,7 @@ static void __init initramfs_test_fname_path_max(struct kunit *test)
 	memcpy(tbufs->fname_oversize, "fname_oversize",
 	       sizeof("fname_oversize") - 1);
 	memcpy(tbufs->fname_ok, "fname_ok", sizeof("fname_ok") - 1);
-	len = fill_cpio(c, ARRAY_SIZE(c), tbufs->cpio_src);
+	len = fill_cpio(c, ARRAY_SIZE(c), false, tbufs->cpio_src);
 
 	/* unpack skips over fname_oversize instead of returning an error */
 	err = unpack_to_rootfs(tbufs->cpio_src, len);
-- 
2.51.0

[PATCH v4 2/6] initramfs_test: test header fields with 0x hex prefix

[David Disseldorp]

cpio header fields are 8-byte hex strings, but one "interesting"
side-effect of our historic simple_str[n]toul() use means that a "0x"
(or "0X") prefixed header field will be successfully processed when
coupled alongside a 6-byte hex remainder string.

"0x" prefix support is contrary to the initramfs specification at
Documentation/driver-api/early-userspace/buffer-format.rst which states:

  The structure of the cpio_header is as follows (all fields contain
  hexadecimal ASCII numbers fully padded with '0' on the left to the
  full width of the field, for example, the integer 4780 is represented
  by the ASCII string "000012ac"):

Test for this corner case by injecting "0x" prefixes into the uid, gid
and namesize cpio header fields. Confirm that init_stat() returns
matching uid and gid values.

This test can be modified in future to expect unpack_to_rootfs() failure
when header validation is changed to properly follow the specification.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 init/initramfs_test.c | 56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/init/initramfs_test.c b/init/initramfs_test.c
index 3ec7591660e03..4a7a85560ee50 100644
--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -503,6 +503,61 @@ static void __init initramfs_test_fname_path_max(struct kunit *test)
 	kfree(tbufs);
 }
 
+static void __init initramfs_test_hdr_hex(struct kunit *test)
+{
+	char *err;
+	size_t len;
+	struct kstat st0, st1;
+	char fdata[] = "this file data will be unpacked";
+	struct initramfs_test_bufs {
+		char cpio_src[(CPIO_HDRLEN + PATH_MAX + 3 + sizeof(fdata)) * 2];
+	} *tbufs = kzalloc(sizeof(struct initramfs_test_bufs), GFP_KERNEL);
+	struct initramfs_test_cpio c[] = { {
+		.magic = "070701",
+		.ino = 1,
+		.mode = S_IFREG | 0777,
+		.uid = 0x123456,
+		.gid = 0x123457,
+		.nlink = 1,
+		.namesize = sizeof("initramfs_test_hdr_hex_0"),
+		.fname = "initramfs_test_hdr_hex_0",
+		.filesize = sizeof(fdata),
+		.data = fdata,
+	}, {
+		.magic = "070701",
+		.ino = 2,
+		.mode = S_IFDIR | 0777,
+		.uid = 0x000056,
+		.gid = 0x000057,
+		.nlink = 1,
+		.namesize = sizeof("initramfs_test_hdr_hex_1"),
+		.fname = "initramfs_test_hdr_hex_1",
+	} };
+
+	/* inject_ox=true to add "0x" cpio field prefixes */
+	len = fill_cpio(c, ARRAY_SIZE(c), true, tbufs->cpio_src);
+
+	err = unpack_to_rootfs(tbufs->cpio_src, len);
+	KUNIT_EXPECT_NULL(test, err);
+
+	KUNIT_EXPECT_EQ(test, init_stat(c[0].fname, &st0, 0), 0);
+	KUNIT_EXPECT_EQ(test, init_stat(c[1].fname, &st1, 0), 0);
+
+	KUNIT_EXPECT_TRUE(test,
+		uid_eq(st0.uid, make_kuid(current_user_ns(), (uid_t)0x123456)));
+	KUNIT_EXPECT_TRUE(test,
+		gid_eq(st0.gid, make_kgid(current_user_ns(), (gid_t)0x123457)));
+	KUNIT_EXPECT_TRUE(test,
+		uid_eq(st1.uid, make_kuid(current_user_ns(), (uid_t)0x56)));
+	KUNIT_EXPECT_TRUE(test,
+		gid_eq(st1.gid, make_kgid(current_user_ns(), (gid_t)0x57)));
+
+	KUNIT_EXPECT_EQ(test, init_unlink(c[0].fname), 0);
+	KUNIT_EXPECT_EQ(test, init_rmdir(c[1].fname), 0);
+
+	kfree(tbufs);
+}
+
 /*
  * The kunit_case/_suite struct cannot be marked as __initdata as this will be
  * used in debugfs to retrieve results after test has run.
@@ -516,6 +571,7 @@ static struct kunit_case __refdata initramfs_test_cases[] = {
 	KUNIT_CASE(initramfs_test_many),
 	KUNIT_CASE(initramfs_test_fname_pad),
 	KUNIT_CASE(initramfs_test_fname_path_max),
+	KUNIT_CASE(initramfs_test_hdr_hex),
 	{},
 };
 
-- 
2.51.0

[PATCH v4 3/6] initramfs: Sort headers alphabetically

[David Disseldorp]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Sorting headers alphabetically helps locating duplicates, and makes it
easier to figure out where to insert new headers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: David Disseldorp <ddiss@suse.de>
---
 init/initramfs.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 139baed06589a..4ed796566cf35 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -1,25 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0
-#include <linux/init.h>
 #include <linux/async.h>
-#include <linux/export.h>
-#include <linux/fs.h>
-#include <linux/slab.h>
-#include <linux/types.h>
-#include <linux/fcntl.h>
 #include <linux/delay.h>
-#include <linux/string.h>
 #include <linux/dirent.h>
-#include <linux/syscalls.h>
-#include <linux/utime.h>
+#include <linux/export.h>
+#include <linux/fcntl.h>
 #include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/init_syscalls.h>
 #include <linux/kstrtox.h>
 #include <linux/memblock.h>
 #include <linux/mm.h>
 #include <linux/namei.h>
-#include <linux/init_syscalls.h>
-#include <linux/umh.h>
-#include <linux/security.h>
 #include <linux/overflow.h>
+#include <linux/security.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/syscalls.h>
+#include <linux/types.h>
+#include <linux/umh.h>
+#include <linux/utime.h>
 
 #include "do_mounts.h"
 #include "initramfs_internal.h"
-- 
2.51.0

[PATCH v4 4/6] initramfs: Refactor to use hex2bin() instead of custom approach

[David Disseldorp]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

There is a simple_strntoul() function used solely as a shortcut
for hex2bin() with proper endianess conversions. Replace that
and drop the unneeded function in the next changes.

This implementation will abort if we fail to parse the cpio header,
instead of using potentially bogus header values.

Co-developed-by: David Disseldorp <ddiss@suse.de>
Signed-off-by: David Disseldorp <ddiss@suse.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 init/initramfs.c      | 44 +++++++++++++++++++++++++------------------
 init/initramfs_test.c | 23 ++++------------------
 2 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 4ed796566cf35..0d38ea8e63a29 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -6,6 +6,7 @@
 #include <linux/fcntl.h>
 #include <linux/file.h>
 #include <linux/fs.h>
+#include <linux/hex.h>
 #include <linux/init.h>
 #include <linux/init_syscalls.h>
 #include <linux/kstrtox.h>
@@ -21,6 +22,8 @@
 #include <linux/umh.h>
 #include <linux/utime.h>
 
+#include <asm/byteorder.h>
+
 #include "do_mounts.h"
 #include "initramfs_internal.h"
 
@@ -190,26 +193,30 @@ static __initdata gid_t gid;
 static __initdata unsigned rdev;
 static __initdata u32 hdr_csum;
 
-static void __init parse_header(char *s)
+static int __init parse_header(char *s)
 {
-	unsigned long parsed[13];
-	int i;
+	__be32 header[13];
+	int ret;
 
-	for (i = 0, s += 6; i < 13; i++, s += 8)
-		parsed[i] = simple_strntoul(s, NULL, 16, 8);
+	ret = hex2bin((u8 *)header, s + 6, sizeof(header));
+	if (ret) {
+		error("damaged header");
+		return ret;
+	}
 
-	ino = parsed[0];
-	mode = parsed[1];
-	uid = parsed[2];
-	gid = parsed[3];
-	nlink = parsed[4];
-	mtime = parsed[5]; /* breaks in y2106 */
-	body_len = parsed[6];
-	major = parsed[7];
-	minor = parsed[8];
-	rdev = new_encode_dev(MKDEV(parsed[9], parsed[10]));
-	name_len = parsed[11];
-	hdr_csum = parsed[12];
+	ino = be32_to_cpu(header[0]);
+	mode = be32_to_cpu(header[1]);
+	uid = be32_to_cpu(header[2]);
+	gid = be32_to_cpu(header[3]);
+	nlink = be32_to_cpu(header[4]);
+	mtime = be32_to_cpu(header[5]); /* breaks in y2106 */
+	body_len = be32_to_cpu(header[6]);
+	major = be32_to_cpu(header[7]);
+	minor = be32_to_cpu(header[8]);
+	rdev = new_encode_dev(MKDEV(be32_to_cpu(header[9]), be32_to_cpu(header[10])));
+	name_len = be32_to_cpu(header[11]);
+	hdr_csum = be32_to_cpu(header[12]);
+	return 0;
 }
 
 /* FSM */
@@ -289,7 +296,8 @@ static int __init do_header(void)
 			error("no cpio magic");
 		return 1;
 	}
-	parse_header(collected);
+	if (parse_header(collected))
+		return 1;
 	next_header = this_header + N_ALIGN(name_len) + body_len;
 	next_header = (next_header + 3) & ~3;
 	state = SkipIt;
diff --git a/init/initramfs_test.c b/init/initramfs_test.c
index 4a7a85560ee50..4785423f601c2 100644
--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -31,7 +31,8 @@ struct initramfs_test_cpio {
 #define CPIO_HDR_FMT "%s%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%s"
 /*
  * bogus newc header with "0x" prefixes on the uid, gid and namesize values.
- * parse_header()/simple_str[n]toul() accept this.
+ * parse_header()/simple_str[n]toul() accepted this, contrary to the initramfs
+ * specification. hex2bin() now fails.
  */
 #define CPIO_HDR_OX_INJECT \
 	"%s%08x%08x0x%06x0X%06x%08x%08x%08x%08x%08x%08x%08x0x%06x%08x%s"
@@ -507,8 +508,7 @@ static void __init initramfs_test_hdr_hex(struct kunit *test)
 {
 	char *err;
 	size_t len;
-	struct kstat st0, st1;
-	char fdata[] = "this file data will be unpacked";
+	char fdata[] = "this file data will not be unpacked";
 	struct initramfs_test_bufs {
 		char cpio_src[(CPIO_HDRLEN + PATH_MAX + 3 + sizeof(fdata)) * 2];
 	} *tbufs = kzalloc(sizeof(struct initramfs_test_bufs), GFP_KERNEL);
@@ -538,22 +538,7 @@ static void __init initramfs_test_hdr_hex(struct kunit *test)
 	len = fill_cpio(c, ARRAY_SIZE(c), true, tbufs->cpio_src);
 
 	err = unpack_to_rootfs(tbufs->cpio_src, len);
-	KUNIT_EXPECT_NULL(test, err);
-
-	KUNIT_EXPECT_EQ(test, init_stat(c[0].fname, &st0, 0), 0);
-	KUNIT_EXPECT_EQ(test, init_stat(c[1].fname, &st1, 0), 0);
-
-	KUNIT_EXPECT_TRUE(test,
-		uid_eq(st0.uid, make_kuid(current_user_ns(), (uid_t)0x123456)));
-	KUNIT_EXPECT_TRUE(test,
-		gid_eq(st0.gid, make_kgid(current_user_ns(), (gid_t)0x123457)));
-	KUNIT_EXPECT_TRUE(test,
-		uid_eq(st1.uid, make_kuid(current_user_ns(), (uid_t)0x56)));
-	KUNIT_EXPECT_TRUE(test,
-		gid_eq(st1.gid, make_kgid(current_user_ns(), (gid_t)0x57)));
-
-	KUNIT_EXPECT_EQ(test, init_unlink(c[0].fname), 0);
-	KUNIT_EXPECT_EQ(test, init_rmdir(c[1].fname), 0);
+	KUNIT_EXPECT_NOT_NULL(test, err);
 
 	kfree(tbufs);
 }
-- 
2.51.0

[PATCH v4 5/6] vsprintf: Revert "add simple_strntoul"

[David Disseldorp]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

No users anymore and none should be in the first place.

This reverts commit fcc155008a20fa31b01569e105250490750f0687.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: David Disseldorp <ddiss@suse.de>
Acked-by: Petr Mladek <pmladek@suse.com>
---
 include/linux/kstrtox.h | 1 -
 lib/vsprintf.c          | 7 -------
 2 files changed, 8 deletions(-)

diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h
index 6ea897222af1d..7fcf29a4e0de4 100644
--- a/include/linux/kstrtox.h
+++ b/include/linux/kstrtox.h
@@ -143,7 +143,6 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
  */
 
 extern unsigned long simple_strtoul(const char *,char **,unsigned int);
-extern unsigned long simple_strntoul(const char *,char **,unsigned int,size_t);
 extern long simple_strtol(const char *,char **,unsigned int);
 extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
 extern long long simple_strtoll(const char *,char **,unsigned int);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 800b8ac49f53f..52ea14a08d3a5 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -129,13 +129,6 @@ unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base)
 }
 EXPORT_SYMBOL(simple_strtoul);
 
-unsigned long simple_strntoul(const char *cp, char **endp, unsigned int base,
-			      size_t max_chars)
-{
-	return simple_strntoull(cp, endp, base, max_chars);
-}
-EXPORT_SYMBOL(simple_strntoul);
-
 /**
  * simple_strtol - convert a string to a signed long
  * @cp: The start of the string
-- 
2.51.0

[PATCH v4 6/6] kstrtox: Drop extern keyword in the simple_strtox() declarations

[David Disseldorp]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

There is legacy 'extern' keyword for the exported simple_strtox()
function which are the artefact that can be removed. So drop it.

While at it, tweak the declaration to provide parameter names.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: David Disseldorp <ddiss@suse.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 include/linux/kstrtox.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h
index 7fcf29a4e0de4..6c92828667704 100644
--- a/include/linux/kstrtox.h
+++ b/include/linux/kstrtox.h
@@ -142,9 +142,9 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
  * Keep in mind above caveat.
  */
 
-extern unsigned long simple_strtoul(const char *,char **,unsigned int);
-extern long simple_strtol(const char *,char **,unsigned int);
-extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
-extern long long simple_strtoll(const char *,char **,unsigned int);
+unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base);
+long simple_strtol(const char *cp, char **endp, unsigned int base);
+unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base);
+long long simple_strtoll(const char *cp, char **endp, unsigned int base);
 
 #endif	/* _LINUX_KSTRTOX_H */
-- 
2.51.0

Re: [PATCH v4 1/6] initramfs_test: add fill_cpio() inject_ox parameter

[Andy Shevchenko]

On Sun, Mar 29, 2026 at 09:30:23PM +1100, David Disseldorp wrote:
> fill_cpio() uses sprintf() to write out the in-memory cpio archive from
> an array of struct initramfs_test_cpio. This change allows callers to
> modify the cpio sprintf() format string so that future tests can
> intentionally corrupt the header with "0x" and "0X" prefixed fields.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> +/*
> + * bogus newc header with "0x" prefixes on the uid, gid and namesize values.

Perhaps Oxford comma?

> + * parse_header()/simple_str[n]toul() accept this.

Also see a comment in patch 4.

> + */

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/6] initramfs: Refactor to use hex2bin() instead of custom approach

[Andy Shevchenko]

On Sun, Mar 29, 2026 at 09:30:26PM +1100, David Disseldorp wrote:

> There is a simple_strntoul() function used solely as a shortcut
> for hex2bin() with proper endianess conversions. Replace that
> and drop the unneeded function in the next changes.
> 
> This implementation will abort if we fail to parse the cpio header,
> instead of using potentially bogus header values.

...

>  /*
>   * bogus newc header with "0x" prefixes on the uid, gid and namesize values.
> - * parse_header()/simple_str[n]toul() accept this.
> + * parse_header()/simple_str[n]toul() accepted this, contrary to the initramfs
> + * specification. hex2bin() now fails.
>   */
>  #define CPIO_HDR_OX_INJECT \
>  	"%s%08x%08x0x%06x0X%06x%08x%08x%08x%08x%08x%08x%08x0x%06x%08x%s"

I think this change should be in patch 1.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 2/6] initramfs_test: test header fields with 0x hex prefix

[Andy Shevchenko]

On Sun, Mar 29, 2026 at 09:30:24PM +1100, David Disseldorp wrote:
> cpio header fields are 8-byte hex strings, but one "interesting"
> side-effect of our historic simple_str[n]toul() use means that a "0x"
> (or "0X") prefixed header field will be successfully processed when
> coupled alongside a 6-byte hex remainder string.
> 
> "0x" prefix support is contrary to the initramfs specification at
> Documentation/driver-api/early-userspace/buffer-format.rst which states:
> 
>   The structure of the cpio_header is as follows (all fields contain
>   hexadecimal ASCII numbers fully padded with '0' on the left to the
>   full width of the field, for example, the integer 4780 is represented
>   by the ASCII string "000012ac"):
> 
> Test for this corner case by injecting "0x" prefixes into the uid, gid
> and namesize cpio header fields. Confirm that init_stat() returns
> matching uid and gid values.
> 
> This test can be modified in future to expect unpack_to_rootfs() failure
> when header validation is changed to properly follow the specification.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/6] initramfs: Refactor to use hex2bin() instead of custom approach

[Andy Shevchenko]

On Mon, Mar 30, 2026 at 12:50:30PM +0300, Andy Shevchenko wrote:
> On Sun, Mar 29, 2026 at 09:30:26PM +1100, David Disseldorp wrote:

...

> >  /*
> >   * bogus newc header with "0x" prefixes on the uid, gid and namesize values.
> > - * parse_header()/simple_str[n]toul() accept this.
> > + * parse_header()/simple_str[n]toul() accepted this, contrary to the initramfs
> > + * specification. hex2bin() now fails.
> >   */
> >  #define CPIO_HDR_OX_INJECT \
> >  	"%s%08x%08x0x%06x0X%06x%08x%08x%08x%08x%08x%08x%08x0x%06x%08x%s"
> 
> I think this change should be in patch 1.

Hmm... Re-reading it, seems the both needs to be updated.

For the patch 1:

 * Bogus newc header with "0x" prefixes on the uid, gid, and namesize values.
 * parse_header()/simple_str[n]toul() accept this, contrary to the initramfs
 * specification.

For the patch 4:

 * Bogus newc header with "0x" prefixes on the uid, gid, and namesize values.
 * parse_header()/simple_str[n]toul() accepted this, contrary to the initramfs
 * specification. hex2bin() now fails.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/6] initramfs: Refactor to use hex2bin() instead of custom approach

[David Disseldorp]

On Mon, 30 Mar 2026 12:55:16 +0300, Andy Shevchenko wrote:

> On Mon, Mar 30, 2026 at 12:50:30PM +0300, Andy Shevchenko wrote:
> > On Sun, Mar 29, 2026 at 09:30:26PM +1100, David Disseldorp wrote:  
> 
> ...
> 
> > >  /*
> > >   * bogus newc header with "0x" prefixes on the uid, gid and namesize values.
> > > - * parse_header()/simple_str[n]toul() accept this.
> > > + * parse_header()/simple_str[n]toul() accepted this, contrary to the initramfs
> > > + * specification. hex2bin() now fails.
> > >   */
> > >  #define CPIO_HDR_OX_INJECT \
> > >  	"%s%08x%08x0x%06x0X%06x%08x%08x%08x%08x%08x%08x%08x0x%06x%08x%s"  
> > 
> > I think this change should be in patch 1.  
> 
> Hmm... Re-reading it, seems the both needs to be updated.
> 
> For the patch 1:
> 
>  * Bogus newc header with "0x" prefixes on the uid, gid, and namesize values.
>  * parse_header()/simple_str[n]toul() accept this, contrary to the initramfs
>  * specification.
> 
> For the patch 4:
> 
>  * Bogus newc header with "0x" prefixes on the uid, gid, and namesize values.
>  * parse_header()/simple_str[n]toul() accepted this, contrary to the initramfs
>  * specification. hex2bin() now fails.

Looks good to me, Andy. I'll wait another day or so for any further
feedback and then send a v5 with this.

Thanks, David

Re: [PATCH v4 4/6] initramfs: Refactor to use hex2bin() instead of custom approach

[Petr Mladek]

On Mon 2026-03-30 12:55:16, Andy Shevchenko wrote:
> On Mon, Mar 30, 2026 at 12:50:30PM +0300, Andy Shevchenko wrote:
> > On Sun, Mar 29, 2026 at 09:30:26PM +1100, David Disseldorp wrote:
> 
> ...
> 
> > >  /*
> > >   * bogus newc header with "0x" prefixes on the uid, gid and namesize values.
> > > - * parse_header()/simple_str[n]toul() accept this.
> > > + * parse_header()/simple_str[n]toul() accepted this, contrary to the initramfs
> > > + * specification. hex2bin() now fails.
> > >   */
> > >  #define CPIO_HDR_OX_INJECT \
> > >  	"%s%08x%08x0x%06x0X%06x%08x%08x%08x%08x%08x%08x%08x0x%06x%08x%s"
> > 
> > I think this change should be in patch 1.
> 
> Hmm... Re-reading it, seems the both needs to be updated.
> 
> For the patch 1:
> 
>  * Bogus newc header with "0x" prefixes on the uid, gid, and namesize values.
>  * parse_header()/simple_str[n]toul() accept this, contrary to the initramfs
>  * specification.
> 
> For the patch 4:
> 
>  * Bogus newc header with "0x" prefixes on the uid, gid, and namesize values.
>  * parse_header()/simple_str[n]toul() accepted this, contrary to the initramfs
>  * specification. hex2bin() now fails.

This looks better to me.

The rest of the patchset looks good to me as well.

Best Regards,
Petr