* [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
* 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
* [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 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