[PATCH bpf 1/2] bpf: Fix NULL-ptr-deref when dumping a key-less BTF hash map

[PATCH bpf 1/2] bpf: Fix NULL-ptr-deref when dumping a key-less BTF hash map

[Jiayuan Chen]

map_check_btf() allows a key-less BTF (btf_key_type_id == 0) only for maps
that have a ->map_check_btf callback, and leaves the actual decision to
that callback. Hash maps used to have no ->map_check_btf, so a key-less
BTF was rejected outright.

That changed once htab and rhtab got a ->map_check_btf to register a dtor:
the callback does not look at the key, so a key-less hash map now passes
map_check_btf() and gets created. Reading it back through bpffs then feeds
the key type_id 0 into btf_type_seq_show(); btf_type_by_id() returns the
void type, kind_ops[BTF_KIND_UNKN] is NULL, and btf_type_show()
dereferences it:

KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
RIP: 0010:btf_type_show (kernel/bpf/btf.c:8251)
Call Trace:
 <TASK>
 btf_type_seq_show_flags (kernel/bpf/btf.c:8269)
 btf_type_seq_show (kernel/bpf/btf.c:8277)
 htab_map_seq_show_elem (kernel/bpf/hashtab.c:1669)
 map_seq_show (kernel/bpf/inode.c:293)
 seq_read_iter (fs/seq_file.c:273)
 seq_read (fs/seq_file.c:163)
 vfs_read (fs/read_write.c:572)
 ksys_read (fs/read_write.c:716)
 __x64_sys_read (fs/read_write.c:725)
 do_syscall_64 (arch/x86/entry/syscall_64.c:61)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
 </TASK>

Only array maps have a use for a key-less BTF (DataSec global data). Reject
it in htab_map_check_btf() and rhtab_map_check_btf(), restoring the
previous behavior, and also let btf_type_show() bail out on a type with no
show op instead of dereferencing NULL, so any other path that reaches it
with a void type degrades gracefully.

Fixes: 1df97a7453ee ("bpf: Register dtor for freeing special fields")
Fixes: 6905f8601298 ("bpf: Allow special fields in resizable hashtab")
Reported-by: syzbot+37b56485bbbf90ad8489@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a8f4e88.27659fcc.2ceef7.0008.GAE@google.com/T/
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 kernel/bpf/btf.c     | 4 ++++
 kernel/bpf/hashtab.c | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 91b8ce77f699..3dbbbac4dd2e 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8248,6 +8248,10 @@ static void btf_type_show(const struct btf *btf, u32 type_id, void *obj,
 	memset(&show->state, 0, sizeof(show->state));
 	memset(&show->obj, 0, sizeof(show->obj));
 
+	/* A void type (e.g. type_id 0) has no show op, don't deref NULL. */
+	if (!t || !btf_type_ops(t))
+		return;
+
 	btf_type_ops(t)->show(btf, t, type_id, obj, 0, show);
 }
 
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index d40cb5dd446c..299859c58b41 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -530,6 +530,10 @@ static int htab_map_check_btf(struct bpf_map *map, const struct btf *btf,
 {
 	struct bpf_htab *htab = container_of(map, struct bpf_htab, map);
 
+	/* Unlike array maps, hash maps have no use for a key-less BTF. */
+	if (btf_type_is_void(key_type))
+		return -EINVAL;
+
 	if (htab_is_prealloc(htab))
 		return 0;
 	/*
@@ -3110,6 +3114,10 @@ static int rhtab_map_check_btf(struct bpf_map *map, const struct btf *btf,
 {
 	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
 
+	/* Unlike array maps, hash maps have no use for a key-less BTF. */
+	if (btf_type_is_void(key_type))
+		return -EINVAL;
+
 	return bpf_ma_set_dtor(map, &rhtab->ma, rhtab_mem_dtor);
 }
 
-- 
2.43.0

[PATCH bpf 2/2] selftests/bpf: Add test for key-less BTF hash map

[Jiayuan Chen]

Create a hash and an rhash map with btf_key_type_id == 0 and expect
bpf_map_create() to fail with -EINVAL. On an unfixed kernel the map is
created; the test then pins and reads it back to walk the bpffs dump path,
which reproduces the btf_type_show() NULL-deref - so running this test on
an unfixed kernel panics it.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 .../bpf/prog_tests/btf_map_keyless.c          | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c b/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c
new file mode 100644
index 000000000000..6e1496cbd8bf
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <test_progs.h>
+#include <bpf/btf.h>
+
+/*
+ * A key-less BTF (btf_key_type_id == 0) is only meant for array maps. A hash
+ * map with one used to NULL-deref in btf_type_show() when dumped via bpffs.
+ * A fixed kernel rejects such a map at creation; on an unfixed kernel the
+ * pin-and-read below deliberately walks that bpffs dump path, so running this
+ * test on an unfixed kernel panics it.
+ */
+static void check_keyless(int map_type, __u32 map_flags, int btf_fd, int val_id)
+{
+	const char *path = "/sys/fs/bpf/keyless_map";
+	LIBBPF_OPTS(bpf_map_create_opts, opts);
+	__u32 key = 1, val = 0x41424344;
+	char buf[256];
+	int map_fd;
+	FILE *f;
+
+	opts.map_flags = map_flags;
+	opts.btf_fd = btf_fd;
+	opts.btf_key_type_id = 0;
+	opts.btf_value_type_id = val_id;
+	map_fd = bpf_map_create(map_type, "keyless_map", 4, 4, 8, &opts);
+
+	if (map_fd >= 0) {
+		/* Unfixed kernel: reproduce the oops via the bpffs dump path. */
+		(void)bpf_map_update_elem(map_fd, &key, &val, 0);
+		if (bpf_obj_pin(map_fd, path) == 0) {
+			f = fopen(path, "r");
+			if (f) {
+				while (fgets(buf, sizeof(buf), f))
+					;
+				fclose(f);
+			}
+			unlink(path);
+		}
+		close(map_fd);
+	}
+
+	ASSERT_EQ(map_fd, -EINVAL, "key-less create is rejected");
+}
+
+void test_btf_map_keyless(void)
+{
+	int btf_fd, val_id;
+	struct btf *btf;
+
+	btf = btf__new_empty();
+	if (!ASSERT_OK_PTR(btf, "btf__new_empty"))
+		return;
+
+	val_id = btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
+	if (!ASSERT_GT(val_id, 0, "btf__add_int"))
+		goto out;
+
+	if (!ASSERT_OK(btf__load_into_kernel(btf), "btf__load_into_kernel"))
+		goto out;
+	btf_fd = btf__fd(btf);
+
+	if (test__start_subtest("hash"))
+		check_keyless(BPF_MAP_TYPE_HASH, 0, btf_fd, val_id);
+	if (test__start_subtest("rhash"))
+		check_keyless(BPF_MAP_TYPE_RHASH, BPF_F_NO_PREALLOC, btf_fd, val_id);
+out:
+	btf__free(btf);
+}
-- 
2.43.0

Re: [PATCH bpf 2/2] selftests/bpf: Add test for key-less BTF hash map

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 6699 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c b/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c
> new file mode 100644
> index 000000000000..6e1496cbd8bf
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <test_progs.h>
> +#include <bpf/btf.h>

This isn't a bug, but tools/testing/selftests/bpf/prog_tests/btf.c already
has a table-driven harness for map-creation-rejection cases. do_test_raw()
(around btf.c:4519-4530) does:

    opts.btf_key_type_id = test->key_type_id;
    opts.btf_value_type_id = test->value_type_id;
    map_fd = bpf_map_create(test->map_type, test->map_name, ...);
    err = ((map_fd < 0) != test->map_create_err);

and struct btf_raw_test already carries .map_type, .key_size, .value_size,
.max_entries, .key_type_id, .value_type_id and .map_create_err, with six
existing .map_create_err = true entries already in that table. A key-less
hash/rhash case could be two more rows in that table rather than a new file.
Is there a reason to create a new file instead of adding cases to the
existing test?

> +
> +/*
> + * A key-less BTF (btf_key_type_id == 0) is only meant for array maps. A hash
> + * map with one used to NULL-deref in btf_type_show() when dumped via bpffs.
> + * A fixed kernel rejects such a map at creation; on an unfixed kernel the
> + * pin-and-read below deliberately walks that bpffs dump path, so running this
> + * test on an unfixed kernel panics it.
> + */
> +static void check_keyless(int map_type, __u32 map_flags, int btf_fd, int val_id)
> +{
> +	const char *path = "/sys/fs/bpf/keyless_map";
> +	LIBBPF_OPTS(bpf_map_create_opts, opts);
> +	__u32 key = 1, val = 0x41424344;
> +	char buf[256];
> +	int map_fd;
> +	FILE *f;
> +
> +	opts.map_flags = map_flags;
> +	opts.btf_fd = btf_fd;
> +	opts.btf_key_type_id = 0;
> +	opts.btf_value_type_id = val_id;
> +	map_fd = bpf_map_create(map_type, "keyless_map", 4, 4, 8, &opts);
> +
> +	if (map_fd >= 0) {
> +		/* Unfixed kernel: reproduce the oops via the bpffs dump path. */
> +		(void)bpf_map_update_elem(map_fd, &key, &val, 0);
> +		if (bpf_obj_pin(map_fd, path) == 0) {
> +			f = fopen(path, "r");
> +			if (f) {
> +				while (fgets(buf, sizeof(buf), f))
> +					;
> +				fclose(f);
> +			}
> +			unlink(path);
> +		}
> +		close(map_fd);
> +	}

Does this block execute the kernel NULL-deref before the assertion can fail?

On any kernel lacking the companion fix f763f12b26fc ("bpf: Fix
NULL-ptr-deref when dumping a key-less BTF hash map") -- every kernel before
this series, any tree bisected between patch 1 and patch 2, and any backport
that takes the selftest without the kernel hunk -- bpf_map_create() succeeds,
so map_fd >= 0 and the block runs. It inserts an element, pins the map into
bpffs, and then reads the pin, which walks exactly the path quoted in the
fix's own splat:

  seq_read -> map_seq_show (kernel/bpf/inode.c)
           -> htab_map_seq_show_elem (kernel/bpf/hashtab.c)
           -> btf_type_seq_show
           -> btf_type_show (kernel/bpf/btf.c:8229)

with map->btf_key_type_id == 0, where btf_type_by_id() yields &btf_void and
kind_ops[BTF_KIND_UNKN] is NULL, so btf_type_ops(t)->show is a
NULL-pointer read.

That is an in-kernel oops in process context: the reading task (the
test_progs process, or one of its -j workers) is killed, so ASSERT_EQ()
below never executes and the harness reports a dead process rather than a
clean subtest failure. On any host booted with panic_on_oops (the norm for
BPF CI / syzkaller configs; tools/testing/selftests/bpf/config.ppc64el
already sets panic_on_warn=1 in CONFIG_CMDLINE) the machine panics and the
remainder of the test_progs run is lost.

ASSERT_EQ(map_fd, -EINVAL, ...) below already fails on an unfixed kernel,
because an unfixed kernel returns a valid fd instead of -EINVAL. On a fixed
kernel this block is unreachable. A selftest should detect the regression,
not trigger it; should the pin-and-read block be dropped (or at most gated
behind an explicit opt-in)?

> +
> +	ASSERT_EQ(map_fd, -EINVAL, "key-less create is rejected");

Does this assertion pass for the right reason?

-EINVAL is the generic BPF_MAP_CREATE rejection code, so this passes for
many reasons that have nothing to do with the key-less BTF check being
present.

Concretely for the "rhash" subtest: on any kernel that does not know
BPF_MAP_TYPE_RHASH (the map type is new in this tree; the selftest is built
from tools/include/uapi/linux/bpf.h, not from the running kernel's),
find_and_alloc_map() returns ERR_PTR(-EINVAL) because bpf_map_types[type] is
NULL or type is out of range. The subtest then passes without ever reaching
rhtab_map_check_btf().

Likewise, rhtab_map_alloc_check() returns -EINVAL for a missing
BPF_F_NO_PREALLOC / bad map_extra, and map_check_btf() returns -EINVAL if
the value type size ever stops matching value_size (4). Any of these makes
the subtest a green tick that pins nothing.

Is there a positive control? The test never asserts that the same
bpf_map_create() call succeeds when btf_key_type_id is set to a real type
id. Adding that (or checking for the create-time BTF log / distinguishing
the failure source) is what separates "the kernel rejected a key-less hash
map" from "the kernel rejected this map for some unrelated reason."

[ ... ]

The commit message states:

> Create a hash and an rhash map with btf_key_type_id == 0 and expect
> bpf_map_create() to fail with -EINVAL. On an unfixed kernel the map is
> created; the test then pins and reads it back to walk the bpffs dump path,
> which reproduces the btf_type_show() NULL-deref - so running this test on
> an unfixed kernel panics it.

Is "panics" accurate? What the traced path actually produces is an oops (a
NULL function-pointer read in btf_type_show() reached from seq_read() in
process context), which makes the reading task dead; the box only panics if
it was booted with panic_on_oops. More importantly, the message presents the
deliberate crash as the intended behaviour of the test rather than
justifying why the reproducer is kept in-tree when ASSERT_EQ(map_fd,
-EINVAL) alone already detects the regression. The commit also carries no
Fixes: tag, which is consistent with it being the selftest half of the
series, but a reviewer reading it in isolation gets no signal that the test
will take down a kernel that lacks the companion patch.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33161022932