Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: David Disseldorp <ddiss@suse.de>
To: linux-fsdevel@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	David Disseldorp <ddiss@suse.de>
Subject: [PATCH v3 8/9] initramfs: fix hardlink hash leak without TRAILER
Date: Thu,  7 Nov 2024 11:17:27 +1100	[thread overview]
Message-ID: <20241107002044.16477-10-ddiss@suse.de> (raw)
In-Reply-To: <20241107002044.16477-1-ddiss@suse.de>

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 0xffff9405408cc000 (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):
    [<0000000066739faa>] __kmalloc_cache_noprof+0x11d/0x250
    [<00000000fc755219>] maybe_link.part.5+0xbc/0x120
    [<000000000526a128>] do_name+0xce/0x2f0
    [<00000000145c1048>] write_buffer+0x22/0x40
    [<000000003f0b4f32>] unpack_to_rootfs+0xf9/0x2a0
    [<00000000d6f7e5af>] initramfs_test_hardlink+0xe3/0x3f0
    [<0000000014fde8d6>] kunit_try_run_case+0x5f/0x130
    [<00000000dc9dafc5>] kunit_generic_run_threadfn_adapter+0x18/0x30
    [<000000001076c239>] kthread+0xc8/0x100
    [<00000000d939f1c1>] ret_from_fork+0x2b/0x40
    [<00000000f848ad1a>] 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 <ddiss@suse.de>
---
 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


  parent reply	other threads:[~2024-11-07  0:22 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-07  0:17 [PATCH v2 0/9] initramfs: kunit tests and cleanups David Disseldorp
2024-11-07  0:17 ` [PATCH v3 1/9] init: add initramfs_internal.h David Disseldorp
2024-11-07  0:17 ` [PATCH v3 2/9] initramfs_test: kunit tests for initramfs unpacking David Disseldorp
2024-11-09  0:25   ` kernel test robot
2024-11-07  0:17 ` [PATCH v3 3/9] vsprintf: add simple_strntoul David Disseldorp
2024-11-07  0:17 ` [PATCH v3 4/9] initramfs: avoid memcpy for hex header fields David Disseldorp
2024-11-07  0:17 ` [PATCH v3 5/9] initramfs: remove extra symlink path buffer David Disseldorp
2024-11-07  0:17 ` [PATCH v3 6/9] initramfs: merge header_buf and name_buf David Disseldorp
2024-11-07  0:17 ` [PATCH v3 7/9] initramfs: reuse name_len for dir mtime tracking David Disseldorp
2024-11-07  0:17 ` David Disseldorp [this message]
2024-11-27  6:35   ` [PATCH v3 8/9] initramfs: fix hardlink hash leak without TRAILER David Disseldorp
2024-11-07  0:17 ` [PATCH v3 9/9] initramfs: avoid static buffer for error message David Disseldorp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241107002044.16477-10-ddiss@suse.de \
    --to=ddiss@suse.de \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox