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

[David Disseldorp <ddiss@suse.de>]

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;