public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] initramfs_test: test header fields with 0x hex prefix
@ 2026-01-20 20:32 David Disseldorp
  2026-01-20 20:32 ` [PATCH 1/2] initramfs_test: add fill_cpio() format parameter David Disseldorp
  2026-01-20 20:32 ` [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix David Disseldorp
  0 siblings, 2 replies; 9+ messages in thread
From: David Disseldorp @ 2026-01-20 20:32 UTC (permalink / raw)
  To: Andy Shevchenko, Christian Brauner; +Cc: Al Viro, linux-fsdevel

As discussed at
https://lore.kernel.org/linux-fsdevel/20260120230030.5813bfb1.ddiss@suse.de
we currently successfully process cpio header fields which include "0x"
hex prefixes. Add a kunit regression test for this corner-case.

This patchset is based atop commit aaf76839616a3cff
("initramfs_test: kunit test for cpio.filesize > PATH_MAX") queued at
https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git .
@Christian: If possible, please fix the commit subject in the above
commit: it should be "cpio.namesize" instead of "cpio.filesize".

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

 init/initramfs_test.c | 88 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 75 insertions(+), 13 deletions(-)


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

* [PATCH 1/2] initramfs_test: add fill_cpio() format parameter
  2026-01-20 20:32 [PATCH 0/2] initramfs_test: test header fields with 0x hex prefix David Disseldorp
@ 2026-01-20 20:32 ` David Disseldorp
  2026-01-21 13:17   ` Andy Shevchenko
  2026-01-20 20:32 ` [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix David Disseldorp
  1 sibling, 1 reply; 9+ messages in thread
From: David Disseldorp @ 2026-01-20 20:32 UTC (permalink / raw)
  To: Andy Shevchenko, Christian Brauner
  Cc: Al Viro, linux-fsdevel, 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
override the cpio sprintf() format string so that future tests can
intentionally corrupt the header with non-hex values.

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

diff --git a/init/initramfs_test.c b/init/initramfs_test.c
index beb6e3cf78081..8dd752de16518 100644
--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -27,7 +27,10 @@ struct initramfs_test_cpio {
 	char *data;
 };
 
-static size_t fill_cpio(struct initramfs_test_cpio *cs, size_t csz, char *out)
+#define CPIO_HDR_FMT "%s%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%s"
+
+static size_t fill_cpio(struct initramfs_test_cpio *cs, size_t csz, char *fmt,
+			char *out)
 {
 	int i;
 	size_t off = 0;
@@ -38,9 +41,7 @@ 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, 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 +103,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), CPIO_HDR_FMT, cpio_srcbuf);
 
 	ktime_get_real_ts64(&ts_before);
 	err = unpack_to_rootfs(cpio_srcbuf, len);
@@ -177,7 +178,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), CPIO_HDR_FMT, cpio_srcbuf);
 	/* overwrite trailing fname terminator and padding */
 	suffix_off = len - 1;
 	while (cpio_srcbuf[suffix_off] == '\0') {
@@ -219,7 +220,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), CPIO_HDR_FMT, cpio_srcbuf);
 
 	err = unpack_to_rootfs(cpio_srcbuf, len);
 	KUNIT_EXPECT_NULL(test, err);
@@ -274,7 +275,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), CPIO_HDR_FMT, cpio_srcbuf);
 
 	err = unpack_to_rootfs(cpio_srcbuf, len);
 	KUNIT_EXPECT_NULL(test, err);
@@ -284,7 +285,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), CPIO_HDR_FMT, cpio_srcbuf);
 
 	err = unpack_to_rootfs(cpio_srcbuf, len);
 	KUNIT_EXPECT_NOT_NULL(test, err);
@@ -330,7 +331,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), CPIO_HDR_FMT, cpio_srcbuf);
 
 	err = unpack_to_rootfs(cpio_srcbuf, len);
 	KUNIT_EXPECT_NULL(test, err);
@@ -371,7 +372,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, CPIO_HDR_FMT, p);
 	}
 
 	len = p - cpio_srcbuf;
@@ -425,7 +426,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), CPIO_HDR_FMT, tbufs->cpio_srcbuf);
 
 	err = unpack_to_rootfs(tbufs->cpio_srcbuf, len);
 	KUNIT_EXPECT_NULL(test, err);
@@ -481,7 +482,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), CPIO_HDR_FMT, tbufs->cpio_src);
 
 	/* unpack skips over fname_oversize instead of returning an error */
 	err = unpack_to_rootfs(tbufs->cpio_src, len);
-- 
2.51.0


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

* [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix
  2026-01-20 20:32 [PATCH 0/2] initramfs_test: test header fields with 0x hex prefix David Disseldorp
  2026-01-20 20:32 ` [PATCH 1/2] initramfs_test: add fill_cpio() format parameter David Disseldorp
@ 2026-01-20 20:32 ` David Disseldorp
  2026-01-20 22:18   ` Andy Shevchenko
  1 sibling, 1 reply; 9+ messages in thread
From: David Disseldorp @ 2026-01-20 20:32 UTC (permalink / raw)
  To: Andy Shevchenko, Christian Brauner
  Cc: Al Viro, linux-fsdevel, 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"
prefixed header field will be successfully processed when coupled
alongside a 6-byte hex remainder string.

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.

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

diff --git a/init/initramfs_test.c b/init/initramfs_test.c
index 8dd752de16518..afbdbc0342bca 100644
--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -495,6 +495,66 @@ 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, *fmt;
+	size_t len;
+	struct kstat st0, st1;
+	char fdata[] = "this file data will be unpacked";
+	struct test_fname_path_max {
+		char cpio_src[(CPIO_HDRLEN + PATH_MAX + 3 + sizeof(fdata)) * 2];
+	} *tbufs = kzalloc(sizeof(struct test_fname_path_max), 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",
+	} };
+	/*
+	 * override CPIO_HDR_FMT and instead use a format string which places
+	 * "0x" prefixes on the uid, gid and namesize values.
+	 * parse_header()/simple_str[n]toul() accept this.
+	 */
+	fmt = "%s%08x%08x0x%06x0x%06x%08x%08x%08x%08x%08x%08x%08x0x%06x%08x%s";
+	len = fill_cpio(c, ARRAY_SIZE(c), fmt, tbufs->cpio_src);
+
+	/* unpack skips over fname_oversize instead of returning an error */
+	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.
@@ -508,6 +568,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


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

* Re: [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix
  2026-01-20 20:32 ` [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix David Disseldorp
@ 2026-01-20 22:18   ` Andy Shevchenko
  2026-01-21  9:42     ` David Disseldorp
  0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2026-01-20 22:18 UTC (permalink / raw)
  To: David Disseldorp; +Cc: Christian Brauner, Al Viro, linux-fsdevel

On Wed, Jan 21, 2026 at 07:32:33AM +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"
> prefixed header field will be successfully processed when coupled
> alongside a 6-byte hex remainder string.

Should mention that this is against specifications.

> 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 is should be considered as an invalid case and I don't believe
we ever had that bad header somewhere. The specification is clear
that the number has to be filled with '0' to the most significant
byte until all 8 positions are filled.

If any test case like this appears it should not be fatal.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix
  2026-01-20 22:18   ` Andy Shevchenko
@ 2026-01-21  9:42     ` David Disseldorp
  2026-01-21 13:15       ` Andy Shevchenko
  0 siblings, 1 reply; 9+ messages in thread
From: David Disseldorp @ 2026-01-21  9:42 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Christian Brauner, Al Viro, linux-fsdevel

On Wed, 21 Jan 2026 00:18:31 +0200, Andy Shevchenko wrote:

> On Wed, Jan 21, 2026 at 07:32:33AM +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"
> > prefixed header field will be successfully processed when coupled
> > alongside a 6-byte hex remainder string.  
> 
> Should mention that this is against specifications.
> 
> > 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 is should be considered as an invalid case and I don't believe
> we ever had that bad header somewhere. The specification is clear
> that the number has to be filled with '0' to the most significant
> byte until all 8 positions are filled.
> 
> If any test case like this appears it should not be fatal.

Yes, the test case can easily be changed to expect an unpack_to_rootfs()
error (or dropped completely). The purpose is just to ensure that the
user visible change is a concious decision rather than an undocumented
side effect.

Cheers, David

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

* Re: [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix
  2026-01-21  9:42     ` David Disseldorp
@ 2026-01-21 13:15       ` Andy Shevchenko
  2026-01-21 16:17         ` David Disseldorp
  0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2026-01-21 13:15 UTC (permalink / raw)
  To: David Disseldorp; +Cc: Christian Brauner, Al Viro, linux-fsdevel

On Wed, Jan 21, 2026 at 08:42:05PM +1100, David Disseldorp wrote:
> On Wed, 21 Jan 2026 00:18:31 +0200, Andy Shevchenko wrote:
> > On Wed, Jan 21, 2026 at 07:32:33AM +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"
> > > prefixed header field will be successfully processed when coupled
> > > alongside a 6-byte hex remainder string.  
> > 
> > Should mention that this is against specifications.
> > 
> > > 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 is should be considered as an invalid case and I don't believe
> > we ever had that bad header somewhere. The specification is clear
> > that the number has to be filled with '0' to the most significant
> > byte until all 8 positions are filled.
> > 
> > If any test case like this appears it should not be fatal.
> 
> Yes, the test case can easily be changed to expect an unpack_to_rootfs()
> error (or dropped completely). The purpose is just to ensure that the
> user visible change is a concious decision rather than an undocumented
> side effect.

Can you say this clearly in the commit message? With that done I will have
no objections as it seems we all agree with the possible breakage of this
"feature" (implementation detail).

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 1/2] initramfs_test: add fill_cpio() format parameter
  2026-01-20 20:32 ` [PATCH 1/2] initramfs_test: add fill_cpio() format parameter David Disseldorp
@ 2026-01-21 13:17   ` Andy Shevchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2026-01-21 13:17 UTC (permalink / raw)
  To: David Disseldorp; +Cc: Christian Brauner, Al Viro, linux-fsdevel

On Wed, Jan 21, 2026 at 07:32:32AM +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
> override the cpio sprintf() format string so that future tests can
> intentionally corrupt the header with non-hex values.

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

Assuming we agreed with the fact that the followed user is non-standard
and against the current specifications and may be changed in the future.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix
  2026-01-21 13:15       ` Andy Shevchenko
@ 2026-01-21 16:17         ` David Disseldorp
  2026-01-21 16:30           ` Andy Shevchenko
  0 siblings, 1 reply; 9+ messages in thread
From: David Disseldorp @ 2026-01-21 16:17 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Christian Brauner, Al Viro, linux-fsdevel

On Wed, 21 Jan 2026 15:15:54 +0200, Andy Shevchenko wrote:

> On Wed, Jan 21, 2026 at 08:42:05PM +1100, David Disseldorp wrote:
> > On Wed, 21 Jan 2026 00:18:31 +0200, Andy Shevchenko wrote:  
> > > On Wed, Jan 21, 2026 at 07:32:33AM +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"
> > > > prefixed header field will be successfully processed when coupled
> > > > alongside a 6-byte hex remainder string.    
> > > 
> > > Should mention that this is against specifications.

I've added this and will send as v2.

> > > > 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 is should be considered as an invalid case and I don't believe
> > > we ever had that bad header somewhere. The specification is clear
> > > that the number has to be filled with '0' to the most significant
> > > byte until all 8 positions are filled.
> > > 
> > > If any test case like this appears it should not be fatal.  
> > 
> > Yes, the test case can easily be changed to expect an unpack_to_rootfs()
> > error (or dropped completely). The purpose is just to ensure that the
> > user visible change is a concious decision rather than an undocumented
> > side effect.  
> 
> Can you say this clearly in the commit message? With that done I will have
> no objections as it seems we all agree with the possible breakage of this
> "feature" (implementation detail).

Sure, I think it'd make sense to put the v2 test patches as 1/2 in your
series such that your subsequent hex2bin() patch modifies the test to
expect error. E.g.

--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -499,8 +499,7 @@ static void __init initramfs_test_hdr_hex(struct kunit *test)
 {
        char *err, *fmt;
        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);
@@ -528,28 +527,14 @@ static void __init initramfs_test_hdr_hex(struct kunit *test)
        /*
         * override CPIO_HDR_FMT and instead use a format string which places
         * "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.
         */
        fmt = "%s%08x%08x0x%06x0X%06x%08x%08x%08x%08x%08x%08x%08x0x%06x%08x%s";
        len = fill_cpio(c, ARRAY_SIZE(c), fmt, 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);

IMO the only thing then missing is proper
hex2bin->parse_header->do_header error propagation, e.g.

--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -193,14 +193,16 @@ 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)
 {
        __be32 header[13];
        int ret;
 
        ret = hex2bin((u8 *)header, s + 6, sizeof(header));
-       if (ret)
+       if (ret) {
                error("damaged header");
+               return ret;
+       }
 
        ino = be32_to_cpu(header[0]);
        mode = be32_to_cpu(header[1]);
@@ -214,6 +216,7 @@ static void __init parse_header(char *s)
        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 */
@@ -293,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;


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

* Re: [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix
  2026-01-21 16:17         ` David Disseldorp
@ 2026-01-21 16:30           ` Andy Shevchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2026-01-21 16:30 UTC (permalink / raw)
  To: David Disseldorp; +Cc: Christian Brauner, Al Viro, linux-fsdevel

On Thu, Jan 22, 2026 at 03:17:02AM +1100, David Disseldorp wrote:
> On Wed, 21 Jan 2026 15:15:54 +0200, Andy Shevchenko wrote:
> > On Wed, Jan 21, 2026 at 08:42:05PM +1100, David Disseldorp wrote:
> > > On Wed, 21 Jan 2026 00:18:31 +0200, Andy Shevchenko wrote:  
> > > > On Wed, Jan 21, 2026 at 07:32:33AM +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"
> > > > > prefixed header field will be successfully processed when coupled
> > > > > alongside a 6-byte hex remainder string.    
> > > > 
> > > > Should mention that this is against specifications.
> 
> I've added this and will send as v2.

Thanks!

> > > > > 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 is should be considered as an invalid case and I don't believe
> > > > we ever had that bad header somewhere. The specification is clear
> > > > that the number has to be filled with '0' to the most significant
> > > > byte until all 8 positions are filled.
> > > > 
> > > > If any test case like this appears it should not be fatal.  
> > > 
> > > Yes, the test case can easily be changed to expect an unpack_to_rootfs()
> > > error (or dropped completely). The purpose is just to ensure that the
> > > user visible change is a concious decision rather than an undocumented
> > > side effect.  
> > 
> > Can you say this clearly in the commit message? With that done I will have
> > no objections as it seems we all agree with the possible breakage of this
> > "feature" (implementation detail).
> 
> Sure, I think it'd make sense to put the v2 test patches as 1/2 in your
> series such that your subsequent hex2bin() patch modifies the test to
> expect error. E.g.

Yes! Thanks for providing a PoC. Maybe you can do it as a formal patch that
I can simply take into my next version?

-- 
With Best Regards,
Andy Shevchenko



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

end of thread, other threads:[~2026-01-21 16:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-20 20:32 [PATCH 0/2] initramfs_test: test header fields with 0x hex prefix David Disseldorp
2026-01-20 20:32 ` [PATCH 1/2] initramfs_test: add fill_cpio() format parameter David Disseldorp
2026-01-21 13:17   ` Andy Shevchenko
2026-01-20 20:32 ` [PATCH 2/2] initramfs_test: test header fields with 0x hex prefix David Disseldorp
2026-01-20 22:18   ` Andy Shevchenko
2026-01-21  9:42     ` David Disseldorp
2026-01-21 13:15       ` Andy Shevchenko
2026-01-21 16:17         ` David Disseldorp
2026-01-21 16:30           ` Andy Shevchenko

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