[PATCH v2 0/9] initramfs: kunit tests and cleanups
This patchset adds basic kunit test coverage for initramfs unpacking and cleans up some minor buffer handling issues / inefficiencies. Changes since v2 (patch 2 only): - fix !CONFIG_INITRAMFS_PRESERVE_MTIME kunit test checks - add test MODULE_DESCRIPTION(), as suggested by Jeff Johnson - add some missing headers, reported by kernel test robot Changes since v1 (RFC): - rebase atop v6.12-rc6 and filename field overrun fix from https://lore.kernel.org/r/20241030035509.20194-2-dd...@suse.de - add unit test coverage (new patches 1 and 2) - add patch: fix hardlink hash leak without TRAILER - rework patch: avoid static buffer for error message + drop unnecessary message propagation - drop patch: cpio_buf reuse for built-in and bootloader initramfs + no good justification for the change Feedback appreciated. David Disseldorp (9): init: add initramfs_internal.h initramfs_test: kunit tests for initramfs unpacking vsprintf: add simple_strntoul initramfs: avoid memcpy for hex header fields initramfs: remove extra symlink path buffer initramfs: merge header_buf and name_buf initramfs: reuse name_len for dir mtime tracking initramfs: fix hardlink hash leak without TRAILER initramfs: avoid static buffer for error message include/linux/kstrtox.h | 1 + init/.kunitconfig | 3 + init/Kconfig | 7 + init/Makefile | 1 + init/initramfs.c | 73 + init/initramfs_internal.h | 8 + init/initramfs_test.c | 403 ++ lib/vsprintf.c| 7 + 8 files changed, 471 insertions(+), 32 deletions(-) create mode 100644 init/.kunitconfig create mode 100644 init/initramfs_internal.h create mode 100644 init/initramfs_test.c
[PATCH v3 1/9] init: add initramfs_internal.h
The new header only exports a single unpack function and a CPIO_HDRLEN
constant for future test use.
Signed-off-by: David Disseldorp
---
init/initramfs.c | 16 +---
init/initramfs_internal.h | 8
2 files changed, 21 insertions(+), 3 deletions(-)
create mode 100644 init/initramfs_internal.h
diff --git a/init/initramfs.c b/init/initramfs.c
index b2f7583bb1f5c..002e83ae12ac7 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -20,6 +20,7 @@
#include
#include "do_mounts.h"
+#include "initramfs_internal.h"
static __initdata bool csum_present;
static __initdata u32 io_csum;
@@ -256,7 +257,7 @@ static __initdata char *header_buf, *symlink_buf, *name_buf;
static int __init do_start(void)
{
- read_into(header_buf, 110, GotHeader);
+ read_into(header_buf, CPIO_HDRLEN, GotHeader);
return 0;
}
@@ -497,14 +498,23 @@ static unsigned long my_inptr __initdata; /* index of
next byte to be processed
#include
-static char * __init unpack_to_rootfs(char *buf, unsigned long len)
+/**
+ * unpack_to_rootfs - decompress and extract an initramfs archive
+ * @buf: input initramfs archive to extract
+ * @len: length of initramfs data to process
+ *
+ * Returns: NULL for success or an error message string
+ *
+ * This symbol shouldn't be used externally. It's available for unit tests.
+ */
+char * __init unpack_to_rootfs(char *buf, unsigned long len)
{
long written;
decompress_fn decompress;
const char *compress_name;
static __initdata char msg_buf[64];
- header_buf = kmalloc(110, GFP_KERNEL);
+ header_buf = kmalloc(CPIO_HDRLEN, GFP_KERNEL);
symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL);
name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
diff --git a/init/initramfs_internal.h b/init/initramfs_internal.h
new file mode 100644
index 0..233dad16b0a00
--- /dev/null
+++ b/init/initramfs_internal.h
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+#ifndef __INITRAMFS_INTERNAL_H__
+#define __INITRAMFS_INTERNAL_H__
+
+char *unpack_to_rootfs(char *buf, unsigned long len);
+#define CPIO_HDRLEN 110
+
+#endif
--
2.43.0
[PATCH v3 2/9] initramfs_test: kunit tests for initramfs unpacking
Provide some basic initramfs unpack sanity tests covering:
- simple file / dir extraction
- filename field overrun, as reported and fixed separately via
https://lore.kernel.org/r/20241030035509.20194-2-dd...@suse.de
- "070702" cpio data checksums
- hardlinks
Signed-off-by: David Disseldorp
---
init/.kunitconfig | 3 +
init/Kconfig | 7 +
init/Makefile | 1 +
init/initramfs_test.c | 408 ++
4 files changed, 419 insertions(+)
create mode 100644 init/.kunitconfig
create mode 100644 init/initramfs_test.c
diff --git a/init/.kunitconfig b/init/.kunitconfig
new file mode 100644
index 0..acb906b1a5f98
--- /dev/null
+++ b/init/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_INITRAMFS_TEST=y
diff --git a/init/Kconfig b/init/Kconfig
index c521e1421ad4a..cf8327cdd6739 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1431,6 +1431,13 @@ config INITRAMFS_PRESERVE_MTIME
If unsure, say Y.
+config INITRAMFS_TEST
+ bool "Test initramfs cpio archive extraction" if !KUNIT_ALL_TESTS
+ depends on BLK_DEV_INITRD && KUNIT=y
+ default KUNIT_ALL_TESTS
+ help
+ Build KUnit tests for initramfs. See Documentation/dev-tools/kunit
+
choice
prompt "Compiler optimization level"
default CC_OPTIMIZE_FOR_PERFORMANCE
diff --git a/init/Makefile b/init/Makefile
index 10b652d33e872..d6f75d8907e09 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -12,6 +12,7 @@ else
obj-$(CONFIG_BLK_DEV_INITRD) += initramfs.o
endif
obj-$(CONFIG_GENERIC_CALIBRATE_DELAY) += calibrate.o
+obj-$(CONFIG_INITRAMFS_TEST) += initramfs_test.o
obj-y += init_task.o
diff --git a/init/initramfs_test.c b/init/initramfs_test.c
new file mode 100644
index 0..84b21f465bc3d
--- /dev/null
+++ b/init/initramfs_test.c
@@ -0,0 +1,408 @@
+// SPDX-License-Identifier: GPL-2.0
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "initramfs_internal.h"
+
+struct initramfs_test_cpio {
+ char *magic;
+ unsigned int ino;
+ unsigned int mode;
+ unsigned int uid;
+ unsigned int gid;
+ unsigned int nlink;
+ unsigned int mtime;
+ unsigned int filesize;
+ unsigned int devmajor;
+ unsigned int devminor;
+ unsigned int rdevmajor;
+ unsigned int rdevminor;
+ unsigned int namesize;
+ unsigned int csum;
+ char *fname;
+ char *data;
+};
+
+static size_t fill_cpio(struct initramfs_test_cpio *cs, size_t csz, char *out)
+{
+ int i;
+ size_t off = 0;
+
+ for (i = 0; i < csz; i++) {
+ char *pos = &out[off];
+ struct initramfs_test_cpio *c = &cs[i];
+ 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",
+ 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,
+ c->fname) + 1;
+ pr_debug("packing (%zu): %.*s\n", thislen, (int)thislen, pos);
+ off += thislen;
+ while (off & 3)
+ out[off++] = '\0';
+
+ memcpy(&out[off], c->data, c->filesize);
+ off += c->filesize;
+ while (off & 3)
+ out[off++] = '\0';
+ }
+
+ return off;
+}
+
+static void __init initramfs_test_extract(struct kunit *test)
+{
+ char *err, *cpio_srcbuf;
+ size_t len;
+ struct timespec64 ts_before, ts_after;
+ struct kstat st = {};
+ struct initramfs_test_cpio c[] = { {
+ .magic = "070701",
+ .ino = 1,
+ .mode = S_IFREG | 0777,
+ .uid = 12,
+ .gid = 34,
+ .nlink = 1,
+ .mtime = 56,
+ .filesize = 0,
+ .devmajor = 0,
+ .devminor = 1,
+ .rdevmajor = 0,
+ .rdevminor = 0,
+ .namesize = sizeof("initramfs_test_extract"),
+ .csum = 0,
+ .fname = "initramfs_test_extract",
+ }, {
+ .magic = "070701",
+ .ino = 2,
+ .mode = S_IFDIR | 0777,
+ .nlink = 1,
+ .mtime = 57,
+ .devminor = 1,
+ .namesize = sizeof("initramfs_test_extract_dir"),
+ .fname = "initramfs_test_extract_di
[PATCH v3 3/9] vsprintf: add simple_strntoul
cpio extraction currently does a memcpy to ensure that the archive hex
fields are null terminated for simple_strtoul(). simple_strntoul() will
allow us to avoid the memcpy.
Signed-off-by: David Disseldorp
---
include/linux/kstrtox.h | 1 +
lib/vsprintf.c | 7 +++
2 files changed, 8 insertions(+)
diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h
index 7fcf29a4e0de4..6ea897222af1d 100644
--- a/include/linux/kstrtox.h
+++ b/include/linux/kstrtox.h
@@ -143,6 +143,7 @@ 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 c5e2ec9303c5d..32eacaae97990 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -114,6 +114,13 @@ 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.43.0
[PATCH v3 6/9] initramfs: merge header_buf and name_buf
header_buf is only used in FSM states up to GotHeader, while name_buf is
only used in states following GotHeader (Collect is shared, but the
collect pointer tracks each buffer). These buffers can therefore be
combined into a single cpio_buf, which can be used for both header and
file name storage.
Signed-off-by: David Disseldorp
---
init/initramfs.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index 59b4a43fa491b..4e2506a4bc76f 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -250,11 +250,11 @@ static void __init read_into(char *buf, unsigned size,
enum state next)
}
}
-static __initdata char *header_buf, *name_buf;
+static __initdata char *cpio_buf;
static int __init do_start(void)
{
- read_into(header_buf, CPIO_HDRLEN, GotHeader);
+ read_into(cpio_buf, CPIO_HDRLEN, GotHeader);
return 0;
}
@@ -294,14 +294,14 @@ static int __init do_header(void)
if (S_ISLNK(mode)) {
if (body_len > PATH_MAX)
return 0;
- collect = collected = name_buf;
+ collect = collected = cpio_buf;
remains = N_ALIGN(name_len) + body_len;
next_state = GotSymlink;
state = Collect;
return 0;
}
if (S_ISREG(mode) || !body_len)
- read_into(name_buf, N_ALIGN(name_len), GotName);
+ read_into(cpio_buf, N_ALIGN(name_len), GotName);
return 0;
}
@@ -511,11 +511,16 @@ char * __init unpack_to_rootfs(char *buf, unsigned long
len)
const char *compress_name;
static __initdata char msg_buf[64];
- header_buf = kmalloc(CPIO_HDRLEN, GFP_KERNEL);
- /* 2x PATH_MAX as @name_buf is also used for staging symlink targets */
- name_buf = kmalloc(N_ALIGN(PATH_MAX) + PATH_MAX + 1, GFP_KERNEL);
- if (!header_buf || !name_buf)
- panic_show_mem("can't allocate buffers");
+ /*
+* @cpio_buf can be used for staging the 110 byte newc/crc cpio header,
+* after which parse_header() converts and stashes fields into
+* corresponding types. The same buffer can then be reused for file
+* path staging. 2 x PATH_MAX covers any possible symlink target.
+*/
+ BUILD_BUG_ON(CPIO_HDRLEN > N_ALIGN(PATH_MAX) + PATH_MAX + 1);
+ cpio_buf = kmalloc(N_ALIGN(PATH_MAX) + PATH_MAX + 1, GFP_KERNEL);
+ if (!cpio_buf)
+ panic_show_mem("can't allocate buffer");
state = Start;
this_header = 0;
@@ -559,8 +564,7 @@ char * __init unpack_to_rootfs(char *buf, unsigned long len)
len -= my_inptr;
}
dir_utime();
- kfree(name_buf);
- kfree(header_buf);
+ kfree(cpio_buf);
return message;
}
--
2.43.0
[PATCH v3 7/9] initramfs: reuse name_len for dir mtime tracking
We already have a nulterm-inclusive, checked name_len for the directory
name, so use that instead of calling strlen().
Signed-off-by: David Disseldorp
---
init/initramfs.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index 4e2506a4bc76f..c264f136c5281 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -144,9 +144,8 @@ struct dir_entry {
char name[];
};
-static void __init dir_add(const char *name, time64_t mtime)
+static void __init dir_add(const char *name, size_t nlen, time64_t mtime)
{
- size_t nlen = strlen(name) + 1;
struct dir_entry *de;
de = kmalloc(sizeof(struct dir_entry) + nlen, GFP_KERNEL);
@@ -170,7 +169,7 @@ static void __init dir_utime(void)
#else
static void __init do_utime(char *filename, time64_t mtime) {}
static void __init do_utime_path(const struct path *path, time64_t mtime) {}
-static void __init dir_add(const char *name, time64_t mtime) {}
+static void __init dir_add(const char *name, size_t nlen, time64_t mtime) {}
static void __init dir_utime(void) {}
#endif
@@ -394,7 +393,7 @@ static int __init do_name(void)
init_mkdir(collected, mode);
init_chown(collected, uid, gid, 0);
init_chmod(collected, mode);
- dir_add(collected, mtime);
+ dir_add(collected, name_len, mtime);
} else if (S_ISBLK(mode) || S_ISCHR(mode) ||
S_ISFIFO(mode) || S_ISSOCK(mode)) {
if (maybe_link() == 0) {
--
2.43.0
[PATCH v3 5/9] initramfs: remove extra symlink path buffer
A (newc/crc) cpio entry with mode.S_IFLNK set carries the symlink target
in the cpio data segment, following the padded name_len sized file
path. symlink_buf is heap-allocated for staging both file path and
symlink target, while name_buf is additionally allocated for staging
paths for non-symlink cpio entries.
Separate symlink / non-symlink buffers are unnecessary, so just extend
the size of name_buf and use it for both.
Signed-off-by: David Disseldorp
---
init/initramfs.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index 6dd3b02c15d7e..59b4a43fa491b 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -250,7 +250,7 @@ static void __init read_into(char *buf, unsigned size, enum
state next)
}
}
-static __initdata char *header_buf, *symlink_buf, *name_buf;
+static __initdata char *header_buf, *name_buf;
static int __init do_start(void)
{
@@ -294,7 +294,7 @@ static int __init do_header(void)
if (S_ISLNK(mode)) {
if (body_len > PATH_MAX)
return 0;
- collect = collected = symlink_buf;
+ collect = collected = name_buf;
remains = N_ALIGN(name_len) + body_len;
next_state = GotSymlink;
state = Collect;
@@ -512,10 +512,9 @@ char * __init unpack_to_rootfs(char *buf, unsigned long
len)
static __initdata char msg_buf[64];
header_buf = kmalloc(CPIO_HDRLEN, GFP_KERNEL);
- symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL);
- name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
-
- if (!header_buf || !symlink_buf || !name_buf)
+ /* 2x PATH_MAX as @name_buf is also used for staging symlink targets */
+ name_buf = kmalloc(N_ALIGN(PATH_MAX) + PATH_MAX + 1, GFP_KERNEL);
+ if (!header_buf || !name_buf)
panic_show_mem("can't allocate buffers");
state = Start;
@@ -561,7 +560,6 @@ char * __init unpack_to_rootfs(char *buf, unsigned long len)
}
dir_utime();
kfree(name_buf);
- kfree(symlink_buf);
kfree(header_buf);
return message;
}
--
2.43.0
[PATCH v3 9/9] initramfs: avoid static buffer for error message
The dynamic error message printed if CONFIG_RD_$ALG compression support
is missing needn't be propagated up to the caller via a static buffer.
Print it immediately via pr_err() and set @message to a const string to
flag error.
Before:
textdata bss dec hex filename
76951102 888052265 ./init/initramfs.o
After:
textdata bss dec hex filename
76831006 8869721f9 ./init/initramfs.o
Signed-off-by: David Disseldorp
---
init/initramfs.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index 99f3cac10d392..f946b7680867b 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -511,7 +511,6 @@ char * __init unpack_to_rootfs(char *buf, unsigned long len)
long written;
decompress_fn decompress;
const char *compress_name;
- static __initdata char msg_buf[64];
/*
* @cpio_buf can be used for staging the 110 byte newc/crc cpio header,
@@ -551,12 +550,9 @@ char * __init unpack_to_rootfs(char *buf, unsigned long
len)
if (res)
error("decompressor failed");
} else if (compress_name) {
- if (!message) {
- snprintf(msg_buf, sizeof msg_buf,
-"compression method %s not configured",
-compress_name);
- message = msg_buf;
- }
+ pr_err("compression method %s not configured\n",
+ compress_name);
+ error("decompressor failed");
} else
error("invalid magic at start of compressed archive");
if (state != Reset)
--
2.43.0
[PATCH v3 4/9] initramfs: avoid memcpy for hex header fields
newc/crc cpio headers contain a bunch of 8-character hexadecimal fields
which we convert via simple_strtoul(), following memcpy() into a
zero-terminated stack buffer. The new simple_strntoul() helper allows us
to pass in max_chars=8 to avoid zero-termination and memcpy().
Signed-off-by: David Disseldorp
---
init/initramfs.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index 002e83ae12ac7..6dd3b02c15d7e 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -189,14 +189,11 @@ static __initdata u32 hdr_csum;
static void __init parse_header(char *s)
{
unsigned long parsed[13];
- char buf[9];
int i;
- buf[8] = '\0';
- for (i = 0, s += 6; i < 13; i++, s += 8) {
- memcpy(buf, s, 8);
- parsed[i] = simple_strtoul(buf, NULL, 16);
- }
+ for (i = 0, s += 6; i < 13; i++, s += 8)
+ parsed[i] = simple_strntoul(s, NULL, 16, 8);
+
ino = parsed[0];
mode = parsed[1];
uid = parsed[2];
--
2.43.0
[PATCH v3 8/9] initramfs: fix hardlink hash leak without TRAILER
Covered in Documentation/driver-api/early-userspace/buffer-format.rst ,
initramfs archives can carry an optional "TRAILER!!!" entry which serves
as a boundary for collecting and associating hardlinks with matching
inode and major / minor device numbers.
Although optional, if hardlinks are found in an archive without a
subsequent "TRAILER!!!" entry then the hardlink state hash table is
leaked, e.g. unfixed kernel, with initramfs_test.c hunk applied only:
unreferenced object 0x9405408cc000 (size 8192):
comm "kunit_try_catch", pid 53, jiffies 4294892519
hex dump (first 32 bytes):
01 00 00 00 01 00 00 00 00 00 00 00 ff 81 00 00
00 00 00 00 00 00 00 00 69 6e 69 74 72 61 6d 66 initramf
backtrace (crc a9fb0ee0):
[<66739faa>] __kmalloc_cache_noprof+0x11d/0x250
[<fc755219>] maybe_link.part.5+0xbc/0x120
[<0526a128>] do_name+0xce/0x2f0
[<145c1048>] write_buffer+0x22/0x40
[<3f0b4f32>] unpack_to_rootfs+0xf9/0x2a0
[<d6f7e5af>] initramfs_test_hardlink+0xe3/0x3f0
[<14fde8d6>] kunit_try_run_case+0x5f/0x130
[<dc9dafc5>] kunit_generic_run_threadfn_adapter+0x18/0x30
[<1076c239>] kthread+0xc8/0x100
[<d939f1c1>] ret_from_fork+0x2b/0x40
[<f848ad1a>] ret_from_fork_asm+0x1a/0x30
Fix this by calling free_hash() after initramfs buffer processing in
unpack_to_rootfs(). An extra hardlink_seen global is added as an
optimization to avoid walking the 32 entry hash array unnecessarily.
The expectation is that a "TRAILER!!!" entry will normally be present,
and initramfs hardlinks are uncommon.
There is one user facing side-effect of this fix: hardlinks can
currently be associated across built-in and external initramfs archives,
*if* the built-in initramfs archive lacks a "TRAILER!!!" terminator. I'd
consider this cross-archive association broken, but perhaps it's used.
Signed-off-by: David Disseldorp
---
init/initramfs.c | 7 ++-
init/initramfs_test.c | 5 -
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index c264f136c5281..99f3cac10d392 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -76,6 +76,7 @@ static __initdata struct hash {
struct hash *next;
char name[N_ALIGN(PATH_MAX)];
} *head[32];
+static __initdata bool hardlink_seen;
static inline int hash(int major, int minor, int ino)
{
@@ -109,19 +110,21 @@ static char __init *find_link(int major, int minor, int
ino,
strcpy(q->name, name);
q->next = NULL;
*p = q;
+ hardlink_seen = true;
return NULL;
}
static void __init free_hash(void)
{
struct hash **p, *q;
- for (p = head; p < head + 32; p++) {
+ for (p = head; hardlink_seen && p < head + 32; p++) {
while (*p) {
q = *p;
*p = q->next;
kfree(q);
}
}
+ hardlink_seen = false;
}
#ifdef CONFIG_INITRAMFS_PRESERVE_MTIME
@@ -563,6 +566,8 @@ char * __init unpack_to_rootfs(char *buf, unsigned long len)
len -= my_inptr;
}
dir_utime();
+ /* free any hardlink state collected without optional TRAILER!!! */
+ free_hash();
kfree(cpio_buf);
return message;
}
diff --git a/init/initramfs_test.c b/init/initramfs_test.c
index 84b21f465bc3d..a2930c70cc817 100644
--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -319,11 +319,6 @@ static void __init initramfs_test_hardlink(struct kunit
*test)
.namesize = sizeof("initramfs_test_hardlink_link"),
.fname = "initramfs_test_hardlink_link",
.data = "ASDF",
- }, {
- /* hardlink hashtable leaks when the archive omits a trailer */
- .magic = "070701",
- .namesize = sizeof("TRAILER!!!"),
- .fname = "TRAILER!!!",
} };
cpio_srcbuf = kmalloc(8192, GFP_KERNEL);
--
2.43.0
Re: [PATCH v2 2/9] initramfs_test: kunit tests for initramfs unpacking
[adding kunit lists to cc, see lore link below for context] https://lore.kernel.org/linux-fsdevel/20241104141750.16119-3-dd...@suse.de/ On Wed, 6 Nov 2024 07:04:21 +0800, kernel test robot wrote: ... > All warnings (new ones prefixed by >>, old ones prefixed by <<): > > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0x0 (section: .data) -> initramfs_test_extract > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0x30 (section: .data) -> initramfs_test_fname_overrun > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0x60 (section: .data) -> initramfs_test_data > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0x90 (section: .data) -> initramfs_test_csum > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0xc0 (section: .data) -> initramfs_test_hardlink > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0xf0 (section: .data) -> initramfs_test_many > >> (section: .init.text) If I understand correctly, the kunit_case initramfs_test_cases[] members can't be flagged __initdata, as they need to be present post-init for results queries via debugfs. The remaining -Wimplicit-function-declaration reports are all valid. I'll fix them in v3. Thanks, David
Re: [PATCH v3 8/9] initramfs: fix hardlink hash leak without TRAILER
On Thu, 7 Nov 2024 11:17:27 +1100, David Disseldorp wrote:
> Covered in Documentation/driver-api/early-userspace/buffer-format.rst ,
> initramfs archives can carry an optional "TRAILER!!!" entry which serves
> as a boundary for collecting and associating hardlinks with matching
> inode and major / minor device numbers.
>
> Although optional, if hardlinks are found in an archive without a
> subsequent "TRAILER!!!" entry then the hardlink state hash table is
> leaked
One further leak is possible if extraction ends prior to fput(wfile)
in CopyFile state, e.g. due to lack of data:
nilchar="\0"
data="123456789ABCDEF"
magic="070701"
ino=1
mode=$(( 0100777 ))
uid=0
gid=0
nlink=1
mtime=1
filesize=$(( ${#data} + 20 )) # too much
devmajor=0
devminor=1
rdevmajor=0
rdevminor=0
csum=0
fname="initramfs_test_archive_overrun"
namelen=$(( ${#fname} + 1 ))# plus one to account for terminator
printf "%s%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%s" \
$magic $ino $mode $uid $gid $nlink $mtime $filesize \
$devmajor $devminor $rdevmajor $rdevminor $namelen $csum $fname
termpadlen=$(( 1 + ((4 - ((110 + $namelen) & 3)) % 4) ))
printf "%.s${nilchar}" $(seq 1 $termpadlen)
# $filesize reaches 20 bytes beyond end of data
printf "%s" "$data"
bash data_repro.sh|gzip >> initramfs
unreferenced object 0x8fdb0192e000 (size 176):
comm "kworker/u8:0", pid 11, jiffies 4294892503
hex dump (first 32 bytes):
01 00 00 00 00 00 00 00 00 00 00 00 1e 80 5d 00 ..].
80 7d a1 a7 ff ff ff ff 10 b1 2f 02 db 8f ff ff .}/.
backtrace (crc 807bd733):
[<e68e8b32>] kmem_cache_alloc_noprof+0x11e/0x260
[<a6f24fcd>] alloc_empty_file+0x45/0x120
[<130beec8>] path_openat+0x2f/0xf30
[<24613ad7>] do_filp_open+0xa7/0x110
[<5f4f0158>] file_open_name+0x118/0x180
[<03ed573f>] filp_open+0x27/0x50
[<91ec9e44>] do_name+0xc4/0x2b0
[<8e084ec8>] write_buffer+0x22/0x40
[<2ea2ff4b>] flush_buffer+0x2f/0x90
[<9085f8b5>] gunzip+0x25a/0x310
[<0c1c83c3>] unpack_to_rootfs+0x176/0x2a0
[<c966fda5>] do_populate_rootfs+0x6a/0x180
[<51fb877d>] async_run_entry_fn+0x31/0x120
[<a3ee305f>] process_scheduled_works+0xbe/0x310
[<83c835bb>] worker_thread+0x100/0x240
[<6ea2f0b3>] kthread+0xc8/0x100
Not sure whether others are interested in seeing these kinds of
leak-on-malformed-archive bugs fixed, but I'll send through a v4 with a
fix + unit test for it.
Re: [brauner-github:vfs.all 205/231] WARNING: modpost: vmlinux: section mismatch in reference: initramfs_test_cases+0x0 (section: .data) -> initramfs_test_extract (section: .init.text)
[cc'ing linux-kselftest and kunit-dev] Hi, On Wed, 5 Mar 2025 01:47:55 +0800, kernel test robot wrote: > tree: https://github.com/brauner/linux.git vfs.all > head: ea47e99a3a234837d5fea0d1a20bb2ad1eaa6dd4 > commit: b6736cfccb582b7c016cba6cd484fbcf30d499af [205/231] initramfs_test: > kunit tests for initramfs unpacking > config: x86_64-buildonly-randconfig-002-20250304 > (https://download.01.org/0day-ci/archive/20250305/202503050109.t5ab93hx-...@intel.com/config) > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 > reproduce (this is a W=1 build): > (https://download.01.org/0day-ci/archive/20250305/202503050109.t5ab93hx-...@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version > of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot > | Closes: > https://lore.kernel.org/oe-kbuild-all/202503050109.t5ab93hx-...@intel.com/ > > All warnings (new ones prefixed by >>, old ones prefixed by <<): > > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0x0 (section: .data) -> initramfs_test_extract > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0x30 (section: .data) -> initramfs_test_fname_overrun > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0x60 (section: .data) -> initramfs_test_data > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0x90 (section: .data) -> initramfs_test_csum > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0xc0 (section: .data) -> initramfs_test_hardlink > >> (section: .init.text) > >> WARNING: modpost: vmlinux: section mismatch in reference: > >> initramfs_test_cases+0xf0 (section: .data) -> initramfs_test_many > >> (section: .init.text) These new warnings are covered in the commit message. The kunit_test_init_section_suites() registered tests aren't in the .init section as debugfs entries are retained for results reporting (without an ability to rerun them). IIUC, the __kunit_init_test_suites->CONCATENATE(..., _probe) suffix is intended to suppress the modpost warning - @kunit-dev: any ideas why this isn't working as intended? Thanks, David
[PATCH v4 7/8] initramfs: fix hardlink hash leak without TRAILER
Covered in Documentation/driver-api/early-userspace/buffer-format.rst ,
initramfs archives can carry an optional "TRAILER!!!" entry which serves
as a boundary for collecting and associating hardlinks with matching
inode and major / minor device numbers.
Although optional, if hardlinks are found in an archive without a
subsequent "TRAILER!!!" entry then the hardlink state hash table is
leaked, e.g. unfixed kernel, with initramfs_test.c hunk applied only:
unreferenced object 0x9405408cc000 (size 8192):
comm "kunit_try_catch", pid 53, jiffies 4294892519
hex dump (first 32 bytes):
01 00 00 00 01 00 00 00 00 00 00 00 ff 81 00 00
00 00 00 00 00 00 00 00 69 6e 69 74 72 61 6d 66 initramf
backtrace (crc a9fb0ee0):
[<66739faa>] __kmalloc_cache_noprof+0x11d/0x250
[<fc755219>] maybe_link.part.5+0xbc/0x120
[<0526a128>] do_name+0xce/0x2f0
[<145c1048>] write_buffer+0x22/0x40
[<3f0b4f32>] unpack_to_rootfs+0xf9/0x2a0
[<d6f7e5af>] initramfs_test_hardlink+0xe3/0x3f0
[<14fde8d6>] kunit_try_run_case+0x5f/0x130
[<dc9dafc5>] kunit_generic_run_threadfn_adapter+0x18/0x30
[<1076c239>] kthread+0xc8/0x100
[<d939f1c1>] ret_from_fork+0x2b/0x40
[<f848ad1a>] ret_from_fork_asm+0x1a/0x30
Fix this by calling free_hash() after initramfs buffer processing in
unpack_to_rootfs(). An extra hardlink_seen global is added as an
optimization to avoid walking the 32 entry hash array unnecessarily.
The expectation is that a "TRAILER!!!" entry will normally be present,
and initramfs hardlinks are uncommon.
There is one user facing side-effect of this fix: hardlinks can
currently be associated across built-in and external initramfs archives,
*if* the built-in initramfs archive lacks a "TRAILER!!!" terminator. I'd
consider this cross-archive association broken, but perhaps it's used.
Signed-off-by: David Disseldorp
---
init/initramfs.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index b9cacdc54eaf1..e0b11f8d6f3d6 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -76,6 +76,7 @@ static __initdata struct hash {
struct hash *next;
char name[N_ALIGN(PATH_MAX)];
} *head[32];
+static __initdata bool hardlink_seen;
static inline int hash(int major, int minor, int ino)
{
@@ -109,19 +110,21 @@ static char __init *find_link(int major, int minor, int
ino,
strcpy(q->name, name);
q->next = NULL;
*p = q;
+ hardlink_seen = true;
return NULL;
}
static void __init free_hash(void)
{
struct hash **p, *q;
- for (p = head; p < head + 32; p++) {
+ for (p = head; hardlink_seen && p < head + 32; p++) {
while (*p) {
q = *p;
*p = q->next;
kfree(q);
}
}
+ hardlink_seen = false;
}
#ifdef CONFIG_INITRAMFS_PRESERVE_MTIME
@@ -564,6 +567,8 @@ char * __init unpack_to_rootfs(char *buf, unsigned long len)
len -= my_inptr;
}
dir_utime();
+ /* free any hardlink state collected without optional TRAILER!!! */
+ free_hash();
kfree(bufs);
return message;
}
--
2.43.0
[PATCH v4 5/8] initramfs: allocate heap buffers together
header_buf, symlink_buf and name_buf all share the same lifecycle so
needn't be allocated / freed separately. This change leads to a minor
reduction in .text size:
before:
textdata bss dec hex filename
79141110 890322348 init/initramfs.o
after:
textdata bss dec hex filename
78541110 88972230c init/initramfs.o
A previous iteration of this patch reused a single buffer instead of
three, given that buffer use is state-sequential (GotHeader, GotName,
GotSymlink). However, the slight decrease in heap use during early boot
isn't really worth the extra review complexity.
Link: https://lore.kernel.org/all/20241107002044.16477-7-dd...@suse.de/
Signed-off-by: David Disseldorp
---
init/initramfs.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index 6dd3b02c15d7e..df75624cdefbf 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -510,14 +510,19 @@ char * __init unpack_to_rootfs(char *buf, unsigned long
len)
decompress_fn decompress;
const char *compress_name;
static __initdata char msg_buf[64];
+ struct {
+ char header[CPIO_HDRLEN];
+ char symlink[PATH_MAX + N_ALIGN(PATH_MAX) + 1];
+ char name[N_ALIGN(PATH_MAX)];
+ } *bufs = kmalloc(sizeof(*bufs), GFP_KERNEL);
- header_buf = kmalloc(CPIO_HDRLEN, GFP_KERNEL);
- symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL);
- name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
-
- if (!header_buf || !symlink_buf || !name_buf)
+ if (!bufs)
panic_show_mem("can't allocate buffers");
+ header_buf = bufs->header;
+ symlink_buf = bufs->symlink;
+ name_buf = bufs->name;
+
state = Start;
this_header = 0;
message = NULL;
@@ -560,9 +565,7 @@ char * __init unpack_to_rootfs(char *buf, unsigned long len)
len -= my_inptr;
}
dir_utime();
- kfree(name_buf);
- kfree(symlink_buf);
- kfree(header_buf);
+ kfree(bufs);
return message;
}
--
2.43.0
[PATCH v4 2/8] initramfs_test: kunit tests for initramfs unpacking
Provide some basic initramfs unpack sanity tests covering:
- simple file / dir extraction
- filename field overrun, as reported and fixed separately via
https://lore.kernel.org/r/20241030035509.20194-2-dd...@suse.de
- "070702" cpio data checksums
- hardlinks
These tests introduce new modpost warnings for initramfs_test_cases
section=.data -> section=.init.text run_case() hooks. The
kunit_case/_suite struct cannot be marked as __initdata as this
will be used in debugfs to retrieve results after a test has run.
Signed-off-by: David Disseldorp
---
init/.kunitconfig | 3 +
init/Kconfig | 7 +
init/Makefile | 1 +
init/initramfs_test.c | 407 ++
4 files changed, 418 insertions(+)
create mode 100644 init/.kunitconfig
create mode 100644 init/initramfs_test.c
diff --git a/init/.kunitconfig b/init/.kunitconfig
new file mode 100644
index 0..acb906b1a5f98
--- /dev/null
+++ b/init/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_INITRAMFS_TEST=y
diff --git a/init/Kconfig b/init/Kconfig
index d0d021b3fa3b3..ded64f6c949ed 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1454,6 +1454,13 @@ config INITRAMFS_PRESERVE_MTIME
If unsure, say Y.
+config INITRAMFS_TEST
+ bool "Test initramfs cpio archive extraction" if !KUNIT_ALL_TESTS
+ depends on BLK_DEV_INITRD && KUNIT=y
+ default KUNIT_ALL_TESTS
+ help
+ Build KUnit tests for initramfs. See Documentation/dev-tools/kunit
+
choice
prompt "Compiler optimization level"
default CC_OPTIMIZE_FOR_PERFORMANCE
diff --git a/init/Makefile b/init/Makefile
index 10b652d33e872..d6f75d8907e09 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -12,6 +12,7 @@ else
obj-$(CONFIG_BLK_DEV_INITRD) += initramfs.o
endif
obj-$(CONFIG_GENERIC_CALIBRATE_DELAY) += calibrate.o
+obj-$(CONFIG_INITRAMFS_TEST) += initramfs_test.o
obj-y += init_task.o
diff --git a/init/initramfs_test.c b/init/initramfs_test.c
new file mode 100644
index 0..6231fe0125831
--- /dev/null
+++ b/init/initramfs_test.c
@@ -0,0 +1,407 @@
+// SPDX-License-Identifier: GPL-2.0
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "initramfs_internal.h"
+
+struct initramfs_test_cpio {
+ char *magic;
+ unsigned int ino;
+ unsigned int mode;
+ unsigned int uid;
+ unsigned int gid;
+ unsigned int nlink;
+ unsigned int mtime;
+ unsigned int filesize;
+ unsigned int devmajor;
+ unsigned int devminor;
+ unsigned int rdevmajor;
+ unsigned int rdevminor;
+ unsigned int namesize;
+ unsigned int csum;
+ char *fname;
+ char *data;
+};
+
+static size_t fill_cpio(struct initramfs_test_cpio *cs, size_t csz, char *out)
+{
+ int i;
+ size_t off = 0;
+
+ for (i = 0; i < csz; i++) {
+ char *pos = &out[off];
+ struct initramfs_test_cpio *c = &cs[i];
+ 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",
+ 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,
+ c->fname) + 1;
+ pr_debug("packing (%zu): %.*s\n", thislen, (int)thislen, pos);
+ off += thislen;
+ while (off & 3)
+ out[off++] = '\0';
+
+ memcpy(&out[off], c->data, c->filesize);
+ off += c->filesize;
+ while (off & 3)
+ out[off++] = '\0';
+ }
+
+ return off;
+}
+
+static void __init initramfs_test_extract(struct kunit *test)
+{
+ char *err, *cpio_srcbuf;
+ size_t len;
+ struct timespec64 ts_before, ts_after;
+ struct kstat st = {};
+ struct initramfs_test_cpio c[] = { {
+ .magic = "070701",
+ .ino = 1,
+ .mode = S_IFREG | 0777,
+ .uid = 12,
+ .gid = 34,
+ .nlink = 1,
+ .mtime = 56,
+ .filesize = 0,
+ .devmajor = 0,
+ .devminor = 1,
+ .rdevmajor = 0,
+ .rdevminor = 0,
+ .namesize = sizeof("initramfs_test_extract"),
+ .csum = 0,
+ .fname = "initramfs_test_extract",
+ }, {
+ .magic = "070701",
+
[PATCH v4 1/8] init: add initramfs_internal.h
The new header only exports a single unpack function and a CPIO_HDRLEN
constant for future test use.
Signed-off-by: David Disseldorp
---
init/initramfs.c | 16 +---
init/initramfs_internal.h | 8
2 files changed, 21 insertions(+), 3 deletions(-)
create mode 100644 init/initramfs_internal.h
diff --git a/init/initramfs.c b/init/initramfs.c
index b2f7583bb1f5c..002e83ae12ac7 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -20,6 +20,7 @@
#include
#include "do_mounts.h"
+#include "initramfs_internal.h"
static __initdata bool csum_present;
static __initdata u32 io_csum;
@@ -256,7 +257,7 @@ static __initdata char *header_buf, *symlink_buf, *name_buf;
static int __init do_start(void)
{
- read_into(header_buf, 110, GotHeader);
+ read_into(header_buf, CPIO_HDRLEN, GotHeader);
return 0;
}
@@ -497,14 +498,23 @@ static unsigned long my_inptr __initdata; /* index of
next byte to be processed
#include
-static char * __init unpack_to_rootfs(char *buf, unsigned long len)
+/**
+ * unpack_to_rootfs - decompress and extract an initramfs archive
+ * @buf: input initramfs archive to extract
+ * @len: length of initramfs data to process
+ *
+ * Returns: NULL for success or an error message string
+ *
+ * This symbol shouldn't be used externally. It's available for unit tests.
+ */
+char * __init unpack_to_rootfs(char *buf, unsigned long len)
{
long written;
decompress_fn decompress;
const char *compress_name;
static __initdata char msg_buf[64];
- header_buf = kmalloc(110, GFP_KERNEL);
+ header_buf = kmalloc(CPIO_HDRLEN, GFP_KERNEL);
symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL);
name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
diff --git a/init/initramfs_internal.h b/init/initramfs_internal.h
new file mode 100644
index 0..233dad16b0a00
--- /dev/null
+++ b/init/initramfs_internal.h
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+#ifndef __INITRAMFS_INTERNAL_H__
+#define __INITRAMFS_INTERNAL_H__
+
+char *unpack_to_rootfs(char *buf, unsigned long len);
+#define CPIO_HDRLEN 110
+
+#endif
--
2.43.0
[PATCH v4 4/8] initramfs: avoid memcpy for hex header fields
newc/crc cpio headers contain a bunch of 8-character hexadecimal fields
which we convert via simple_strtoul(), following memcpy() into a
zero-terminated stack buffer. The new simple_strntoul() helper allows us
to pass in max_chars=8 to avoid zero-termination and memcpy().
Signed-off-by: David Disseldorp
---
init/initramfs.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index 002e83ae12ac7..6dd3b02c15d7e 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -189,14 +189,11 @@ static __initdata u32 hdr_csum;
static void __init parse_header(char *s)
{
unsigned long parsed[13];
- char buf[9];
int i;
- buf[8] = '\0';
- for (i = 0, s += 6; i < 13; i++, s += 8) {
- memcpy(buf, s, 8);
- parsed[i] = simple_strtoul(buf, NULL, 16);
- }
+ for (i = 0, s += 6; i < 13; i++, s += 8)
+ parsed[i] = simple_strntoul(s, NULL, 16, 8);
+
ino = parsed[0];
mode = parsed[1];
uid = parsed[2];
--
2.43.0
[PATCH v4 8/8] initramfs: avoid static buffer for error message
The dynamic error message printed if CONFIG_RD_$ALG compression support
is missing needn't be propagated up to the caller via a static buffer.
Print it immediately via pr_err() and set @message to a const string to
flag error.
Before:
textdata bss dec hex filename
80061118 8913223ac init/initramfs.o
After:
textdata bss dec hex filename
79381022 889682308 init/initramfs.o
Signed-off-by: David Disseldorp
---
init/initramfs.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index e0b11f8d6f3d6..72bad44a1d418 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -511,7 +511,6 @@ char * __init unpack_to_rootfs(char *buf, unsigned long len)
long written;
decompress_fn decompress;
const char *compress_name;
- static __initdata char msg_buf[64];
struct {
char header[CPIO_HDRLEN];
char symlink[PATH_MAX + N_ALIGN(PATH_MAX) + 1];
@@ -552,12 +551,9 @@ char * __init unpack_to_rootfs(char *buf, unsigned long
len)
if (res)
error("decompressor failed");
} else if (compress_name) {
- if (!message) {
- snprintf(msg_buf, sizeof msg_buf,
-"compression method %s not configured",
-compress_name);
- message = msg_buf;
- }
+ pr_err("compression method %s not configured\n",
+ compress_name);
+ error("decompressor failed");
} else
error("invalid magic at start of compressed archive");
if (state != Reset)
--
2.43.0
[PATCH v4 0/8] initramfs: kunit tests and cleanups
This patchset adds basic kunit test coverage for initramfs unpacking and cleans up some minor buffer handling issues / inefficiencies. Changes since v3: - Drop shared unpack buffer changes + rework into initramfs: allocate heap buffers together (patch 5/8) + extra review complexity wasn't worth the tiny boot-time heap saving - move hardlink hash leak repro into first initramfs_test patch - add note regarding kunit section=.data -> section=.init.text warning Changes since v2 (patch 2 only): - fix !CONFIG_INITRAMFS_PRESERVE_MTIME kunit test checks - add test MODULE_DESCRIPTION(), as suggested by Jeff Johnson - add some missing headers, reported by kernel test robot Changes since v1 (RFC): - rebase atop v6.12-rc6 and filename field overrun fix from https://lore.kernel.org/r/20241030035509.20194-2-dd...@suse.de - add unit test coverage (new patches 1 and 2) - add patch: fix hardlink hash leak without TRAILER - rework patch: avoid static buffer for error message + drop unnecessary message propagation - drop patch: cpio_buf reuse for built-in and bootloader initramfs + no good justification for the change Feedback appreciated. David Disseldorp (8): init: add initramfs_internal.h initramfs_test: kunit tests for initramfs unpacking vsprintf: add simple_strntoul initramfs: avoid memcpy for hex header fields initramfs: allocate heap buffers together initramfs: reuse name_len for dir mtime tracking initramfs: fix hardlink hash leak without TRAILER initramfs: avoid static buffer for error message include/linux/kstrtox.h | 1 + init/.kunitconfig | 3 + init/Kconfig | 7 + init/Makefile | 1 + init/initramfs.c | 66 init/initramfs_internal.h | 8 + init/initramfs_test.c | 407 ++ lib/vsprintf.c| 7 + 8 files changed, 472 insertions(+), 28 deletions(-) create mode 100644 init/.kunitconfig create mode 100644 init/initramfs_internal.h create mode 100644 init/initramfs_test.c
[PATCH v4 6/8] initramfs: reuse name_len for dir mtime tracking
We already have a nulterm-inclusive, checked name_len for the directory
name, so use that instead of calling strlen().
Signed-off-by: David Disseldorp
---
init/initramfs.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index df75624cdefbf..b9cacdc54eaf1 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -144,9 +144,8 @@ struct dir_entry {
char name[];
};
-static void __init dir_add(const char *name, time64_t mtime)
+static void __init dir_add(const char *name, size_t nlen, time64_t mtime)
{
- size_t nlen = strlen(name) + 1;
struct dir_entry *de;
de = kmalloc(sizeof(struct dir_entry) + nlen, GFP_KERNEL);
@@ -170,7 +169,7 @@ static void __init dir_utime(void)
#else
static void __init do_utime(char *filename, time64_t mtime) {}
static void __init do_utime_path(const struct path *path, time64_t mtime) {}
-static void __init dir_add(const char *name, time64_t mtime) {}
+static void __init dir_add(const char *name, size_t nlen, time64_t mtime) {}
static void __init dir_utime(void) {}
#endif
@@ -394,7 +393,7 @@ static int __init do_name(void)
init_mkdir(collected, mode);
init_chown(collected, uid, gid, 0);
init_chmod(collected, mode);
- dir_add(collected, mtime);
+ dir_add(collected, name_len, mtime);
} else if (S_ISBLK(mode) || S_ISCHR(mode) ||
S_ISFIFO(mode) || S_ISSOCK(mode)) {
if (maybe_link() == 0) {
--
2.43.0
[PATCH v4 3/8] vsprintf: add simple_strntoul
cpio extraction currently does a memcpy to ensure that the archive hex
fields are null terminated for simple_strtoul(). simple_strntoul() will
allow us to avoid the memcpy.
Signed-off-by: David Disseldorp
---
include/linux/kstrtox.h | 1 +
lib/vsprintf.c | 7 +++
2 files changed, 8 insertions(+)
diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h
index 7fcf29a4e0de4..6ea897222af1d 100644
--- a/include/linux/kstrtox.h
+++ b/include/linux/kstrtox.h
@@ -143,6 +143,7 @@ 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 56fe963192926..734bd70c8b9b3 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -114,6 +114,13 @@ 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.43.0
Re: [brauner-github:vfs.all 205/231] WARNING: modpost: vmlinux: section mismatch in reference: initramfs_test_cases+0x0 (section: .data) -> initramfs_test_extract (section: .init.text)
On Wed, 5 Mar 2025 11:47:01 +1100, David Disseldorp wrote: > [cc'ing linux-kselftest and kunit-dev] > > Hi, > > On Wed, 5 Mar 2025 01:47:55 +0800, kernel test robot wrote: > > > tree: https://github.com/brauner/linux.git vfs.all > > head: ea47e99a3a234837d5fea0d1a20bb2ad1eaa6dd4 > > commit: b6736cfccb582b7c016cba6cd484fbcf30d499af [205/231] initramfs_test: > > kunit tests for initramfs unpacking > > config: x86_64-buildonly-randconfig-002-20250304 > > (https://download.01.org/0day-ci/archive/20250305/202503050109.t5ab93hx-...@intel.com/config) > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 > > reproduce (this is a W=1 build): > > (https://download.01.org/0day-ci/archive/20250305/202503050109.t5ab93hx-...@intel.com/reproduce) > > > > If you fix the issue in a separate patch/commit (i.e. not just a new > > version of > > the same patch/commit), kindly add following tags > > | Reported-by: kernel test robot > > | Closes: > > https://lore.kernel.org/oe-kbuild-all/202503050109.t5ab93hx-...@intel.com/ > > > > All warnings (new ones prefixed by >>, old ones prefixed by <<): > > > > >> WARNING: modpost: vmlinux: section mismatch in reference: > > >> initramfs_test_cases+0x0 (section: .data) -> initramfs_test_extract > > >> (section: .init.text) > > >> WARNING: modpost: vmlinux: section mismatch in reference: > > >> initramfs_test_cases+0x30 (section: .data) -> > > >> initramfs_test_fname_overrun (section: .init.text) > > >> WARNING: modpost: vmlinux: section mismatch in reference: > > >> initramfs_test_cases+0x60 (section: .data) -> initramfs_test_data > > >> (section: .init.text) > > >> WARNING: modpost: vmlinux: section mismatch in reference: > > >> initramfs_test_cases+0x90 (section: .data) -> initramfs_test_csum > > >> (section: .init.text) > > >> WARNING: modpost: vmlinux: section mismatch in reference: > > >> initramfs_test_cases+0xc0 (section: .data) -> initramfs_test_hardlink > > >> (section: .init.text) > > >> WARNING: modpost: vmlinux: section mismatch in reference: > > >> initramfs_test_cases+0xf0 (section: .data) -> initramfs_test_many > > >> (section: .init.text) > > These new warnings are covered in the commit message. The > kunit_test_init_section_suites() registered tests aren't in the .init > section as debugfs entries are retained for results reporting (without > an ability to rerun them). > IIUC, the __kunit_init_test_suites->CONCATENATE(..., _probe) suffix is > intended to suppress the modpost warning - @kunit-dev: any ideas why > this isn't working as intended? Stephen Rothwell (cc'ed) mentioned that we might be able to use __refdata for initramfs_test_cases. The __ref* description in init.h does indicate that it's suitable, and I now see that it's present in kunit-example-test.c . I'll propose a patch which can be squashed in with the existing commit. Thanks, David
[PATCH] initramfs_test: flag kunit_case __refdata to suppress warning
We already have a comment regarding why .data -> .init references are
present for initramfs_test_cases[]. This allows us to suppress the
modpost warning.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202503050109.t5ab93hx-...@intel.com/
Suggested-by: Stephen Rothwell
Signed-off-by: David Disseldorp
---
@Christian: feel free to squash this in with commit b6736cfccb582
("initramfs_test: kunit tests for initramfs unpacking") in your
vfs-6.15.initramfs branch if you like (and remove "These tests introduce
new modpost warnings..." from the commit msg).
init/initramfs_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/init/initramfs_test.c b/init/initramfs_test.c
index 6231fe0125831..696fff583ee42 100644
--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -387,7 +387,7 @@ static void __init initramfs_test_many(struct kunit *test)
* The kunit_case/_suite struct cannot be marked as __initdata as this will be
* used in debugfs to retrieve results after test has run.
*/
-static struct kunit_case initramfs_test_cases[] = {
+static struct kunit_case __refdata initramfs_test_cases[] = {
KUNIT_CASE(initramfs_test_extract),
KUNIT_CASE(initramfs_test_fname_overrun),
KUNIT_CASE(initramfs_test_data),
--
2.43.0
