BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: Fix the array map gen_lookup index bounds check
@ 2026-11-15 10:25 Yuan Chen
  2026-11-15 10:25 ` [PATCH bpf-next 1/2] bpf: Use a 32-bit compare in the array map gen_lookups Yuan Chen
  2026-11-15 10:25 ` [PATCH bpf-next 2/2] selftests/bpf: Test the emitted bounds check of the array gen_lookup Yuan Chen
  0 siblings, 2 replies; 5+ messages in thread
From: Yuan Chen @ 2026-11-15 10:25 UTC (permalink / raw)
  To: bpf
  Cc: netdev, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Martin KaFai Lau, Shuah Khan, linux-kselftest,
	Yuan Chen

The inlined bpf_map_lookup_elem() of the array map flavors bounds the
index with a 64-bit JGE against map->max_entries.  The index is a
zero-extended u32, but the immediate of a 64-bit JGE is sign-extended,
so for max_entries >= 0x80000000 the compare is never true.

With bypass_spec_v1 the inlined code then addresses the array out of
bounds; without it the index_mask AND keeps the access in bounds, but an
out-of-range key returns a masked element instead of NULL.

Patch 1 switches the three compares to BPF_JMP32_IMM().  Patch 2 adds a
selftest that checks the emitted compare in the translated program.

Yuan Chen (2):
  bpf: Use a 32-bit compare in the array map gen_lookups
  selftests/bpf: Test the emitted bounds check of the array gen_lookup

 kernel/bpf/arraymap.c                         |  12 +-
 .../bpf/prog_tests/array_gen_lookup_jmp32.c   | 204 ++++++++++++++++++
 2 files changed, 210 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/array_gen_lookup_jmp32.c

-- 
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 1/2] bpf: Use a 32-bit compare in the array map gen_lookups
  2026-11-15 10:25 [PATCH bpf-next 0/2] bpf: Fix the array map gen_lookup index bounds check Yuan Chen
@ 2026-11-15 10:25 ` Yuan Chen
  2026-11-15 11:00   ` bot+bpf-ci
  2026-11-15 10:25 ` [PATCH bpf-next 2/2] selftests/bpf: Test the emitted bounds check of the array gen_lookup Yuan Chen
  1 sibling, 1 reply; 5+ messages in thread
From: Yuan Chen @ 2026-11-15 10:25 UTC (permalink / raw)
  To: bpf
  Cc: netdev, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Martin KaFai Lau, Shuah Khan, linux-kselftest,
	Yuan Chen

array_map_gen_lookup(), percpu_array_map_gen_lookup() and
array_of_map_gen_lookup() bound the inlined index with
BPF_JMP_IMM(BPF_JGE, idx, max_entries, ...).  The index is a
zero-extended u32, but the immediate of a 64-bit JGE is sign-extended,
so for max_entries >= 0x80000000 the compare is never true.

With bypass_spec_v1, an out-of-range key can therefore produce
an address beyond the end of the array:
array->value + index * elem_size.
Without bypass_spec_v1 the index_mask AND keeps the access in bounds,
but such a key returns a masked element instead of NULL.

Creating such a map needs at least a 16 GiB allocation (max_entries
0x80000000, elem_size 8); the out-of-bounds path also needs a
CAP_PERFMON creator or mitigations=off.

Use BPF_JMP32_IMM() for the three compares, as commit 70504de0bb62
("xsk: Use a 32-bit compare in xsk_map_gen_lookup") did for
xsk_map_gen_lookup().

Fixes: b2157399cc98 ("bpf: prevent out-of-bounds speculation")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/bpf/arraymap.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 508706d0c360..e7278e423f8e 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -230,10 +230,10 @@ static int array_map_gen_lookup(struct bpf_map *map, struct bpf_insn *insn_buf)
 	*insn++ = BPF_ALU64_IMM(BPF_ADD, map_ptr, offsetof(struct bpf_array, value));
 	*insn++ = BPF_LDX_MEM(BPF_W, ret, index, 0);
 	if (!map->bypass_spec_v1) {
-		*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 4);
+		*insn++ = BPF_JMP32_IMM(BPF_JGE, ret, map->max_entries, 4);
 		*insn++ = BPF_ALU32_IMM(BPF_AND, ret, array->index_mask);
 	} else {
-		*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 3);
+		*insn++ = BPF_JMP32_IMM(BPF_JGE, ret, map->max_entries, 3);
 	}
 
 	if (is_power_of_2(elem_size)) {
@@ -307,10 +307,10 @@ static int percpu_array_map_gen_lookup(struct bpf_map *map, struct bpf_insn *ins
 
 	*insn++ = BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_2, 0);
 	if (!map->bypass_spec_v1) {
-		*insn++ = BPF_JMP_IMM(BPF_JGE, BPF_REG_0, map->max_entries, 6);
+		*insn++ = BPF_JMP32_IMM(BPF_JGE, BPF_REG_0, map->max_entries, 6);
 		*insn++ = BPF_ALU32_IMM(BPF_AND, BPF_REG_0, array->index_mask);
 	} else {
-		*insn++ = BPF_JMP_IMM(BPF_JGE, BPF_REG_0, map->max_entries, 5);
+		*insn++ = BPF_JMP32_IMM(BPF_JGE, BPF_REG_0, map->max_entries, 5);
 	}
 
 	*insn++ = BPF_ALU64_IMM(BPF_LSH, BPF_REG_0, 3);
@@ -1499,10 +1499,10 @@ static int array_of_map_gen_lookup(struct bpf_map *map,
 	*insn++ = BPF_ALU64_IMM(BPF_ADD, map_ptr, offsetof(struct bpf_array, value));
 	*insn++ = BPF_LDX_MEM(BPF_W, ret, index, 0);
 	if (!map->bypass_spec_v1) {
-		*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 6);
+		*insn++ = BPF_JMP32_IMM(BPF_JGE, ret, map->max_entries, 6);
 		*insn++ = BPF_ALU32_IMM(BPF_AND, ret, array->index_mask);
 	} else {
-		*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 5);
+		*insn++ = BPF_JMP32_IMM(BPF_JGE, ret, map->max_entries, 5);
 	}
 	if (is_power_of_2(elem_size))
 		*insn++ = BPF_ALU64_IMM(BPF_LSH, ret, ilog2(elem_size));
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 2/2] selftests/bpf: Test the emitted bounds check of the array gen_lookup
  2026-11-15 10:25 [PATCH bpf-next 0/2] bpf: Fix the array map gen_lookup index bounds check Yuan Chen
  2026-11-15 10:25 ` [PATCH bpf-next 1/2] bpf: Use a 32-bit compare in the array map gen_lookups Yuan Chen
@ 2026-11-15 10:25 ` Yuan Chen
  2026-11-15 10:47   ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: Yuan Chen @ 2026-11-15 10:25 UTC (permalink / raw)
  To: bpf
  Cc: netdev, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Martin KaFai Lau, Shuah Khan, linux-kselftest,
	Yuan Chen

Dump the translated insns of a program that inlines
bpf_map_lookup_elem() on ARRAY, PERCPU_ARRAY and ARRAY_OF_MAPS and
check that the compare bounding the index is a 32-bit JGE.  A 64-bit
JGE with max_entries >= 0x80000000 never fires (the immediate is
sign-extended), which lets the inlined lookup address the array out of
bounds.  Triggering that needs a 16 GiB map, so check the emitted
compare instead of allocating one.

Also run the program to check the inlined lookup still finds an element
in range and returns NULL for an out-of-range index.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 .../bpf/prog_tests/array_gen_lookup_jmp32.c   | 204 ++++++++++++++++++
 1 file changed, 204 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/array_gen_lookup_jmp32.c

diff --git a/tools/testing/selftests/bpf/prog_tests/array_gen_lookup_jmp32.c b/tools/testing/selftests/bpf/prog_tests/array_gen_lookup_jmp32.c
new file mode 100644
index 000000000000..ba8215506534
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/array_gen_lookup_jmp32.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 KylinSoft Corporation. */
+
+#include <test_progs.h>
+
+/*
+ * The inlined bpf_map_lookup_elem() for the array map flavors bounds the
+ * index with an emitted compare against map->max_entries.  The compare
+ * must be a 32-bit JGE: the index is zero-extended into a 64-bit register
+ * and the immediate of a 64-bit JGE is sign-extended, so for a map with
+ * max_entries >= 0x80000000 the 64-bit compare never fires and the inlined
+ * code addresses the array out of bounds (70504de0bb62 fixed the same
+ * emitted sequence in xsk_map_gen_lookup()).
+ *
+ * Triggering the bug needs a map with 0x80000000 entries, i.e. a 16 GiB
+ * allocation, so check the emitted compare instead: a regression to the
+ * 64-bit form makes the test fail.
+ */
+
+static char log_buf[16 * 1024];
+
+/* Loads a program that inlines bpf_map_lookup_elem() on @map_fd and
+ * returns 2 (XDP_PASS) when the element is found, 1 (XDP_DROP) when the
+ * lookup returns NULL.
+ */
+static int load_lookup_prog(int map_fd, __u32 key)
+{
+	const struct bpf_insn insns[] = {
+		BPF_LD_MAP_FD(BPF_REG_1, map_fd),
+		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+		BPF_ST_MEM(BPF_W, BPF_REG_2, 0, key),
+		BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+		BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+		BPF_MOV64_IMM(BPF_REG_0, 2 /* XDP_PASS */),
+		BPF_JMP_A(1),
+		BPF_MOV64_IMM(BPF_REG_0, 1 /* XDP_DROP */),
+		BPF_EXIT_INSN(),
+	};
+	LIBBPF_OPTS(bpf_prog_load_opts, opts,
+		    .log_buf = log_buf,
+		    .log_size = sizeof(log_buf),
+		    .log_level = 1);
+	int fd;
+
+	fd = bpf_prog_load(BPF_PROG_TYPE_XDP, "gen_lookup_test", "GPL",
+			   insns, ARRAY_SIZE(insns), &opts);
+	if (fd < 0)
+		fprintf(stderr, "load failed: %s\n", log_buf);
+	return fd;
+}
+
+static void check_jmp32(int prog_fd, __u32 max_entries, const char *what)
+{
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info), len;
+	struct bpf_insn *xlated;
+	__u32 i, jge32 = 0, jge64 = 0;
+	int err;
+
+	err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
+	if (!ASSERT_OK(err, "prog_get_info"))
+		return;
+
+	len = info.xlated_prog_len;
+	xlated = calloc(1, len);
+	if (!ASSERT_OK_PTR(xlated, "calloc xlated"))
+		return;
+
+	memset(&info, 0, sizeof(info));
+	info.xlated_prog_len = len;
+	info.xlated_prog_insns = ptr_to_u64(xlated);
+	err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
+	if (!ASSERT_OK(err, "prog_get_info xlated"))
+		goto out;
+
+	for (i = 0; i + 1 < info.xlated_prog_len / (u32)sizeof(*xlated); i++) {
+		if (xlated[i].code != (BPF_LDX | BPF_W | BPF_MEM))
+			continue;
+		if (xlated[i + 1].code != (BPF_JMP32 | BPF_JGE | BPF_K) &&
+		    xlated[i + 1].code != (BPF_JMP | BPF_JGE | BPF_K))
+			continue;
+		/* The bounds check follows the index load and uses its result. */
+		if (xlated[i + 1].dst_reg != xlated[i].dst_reg)
+			continue;
+		if (xlated[i + 1].imm != (int)max_entries)
+			continue;
+
+		if (xlated[i + 1].code == (BPF_JMP32 | BPF_JGE | BPF_K))
+			jge32++;
+		else
+			jge64++;
+	}
+
+	ASSERT_EQ(jge64, 0, "no 64-bit JGE after index load");
+	ASSERT_EQ(jge32, 1, what);
+
+out:
+	free(xlated);
+}
+
+static void run_lookup(int map_fd, __u32 key, bool expect_found)
+{
+	static char data[64];
+	int prog_fd = load_lookup_prog(map_fd, key);
+
+	LIBBPF_OPTS(bpf_test_run_opts, opts,
+		    .data_in = data,
+		    .data_size_in = sizeof(data));
+
+	if (!ASSERT_GE(prog_fd, 0, "prog load"))
+		return;
+
+	if (!ASSERT_OK(bpf_prog_test_run_opts(prog_fd, &opts), "test_run"))
+		goto out;
+
+	if (expect_found)
+		ASSERT_EQ(opts.retval, 2, "in-range lookup found");
+	else
+		ASSERT_EQ(opts.retval, 1, "out-of-range lookup NULL");
+
+out:
+	close(prog_fd);
+}
+
+void test_array_gen_lookup_jmp32(void)
+{
+	const __u32 max_entries = 4;
+	LIBBPF_OPTS(bpf_map_create_opts, copts);
+	LIBBPF_OPTS(bpf_map_create_opts, oopts);
+	int map_fd = -1, pcpu_fd = -1, inner_fd = -1, outer_fd = -1, prog_fd;
+	__u32 key = 1;
+	__u64 val = 0x1234;
+
+	/* The lookup is inlined only for 64-bit programs with the JIT on. */
+	if (!is_jit_enabled() || sizeof(void *) != 8) {
+		test__skip();
+		return;
+	}
+
+	map_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "arr", sizeof(__u32),
+				sizeof(__u64), max_entries, &copts);
+	if (!ASSERT_GE(map_fd, 0, "array map"))
+		goto out;
+	if (!ASSERT_OK(bpf_map_update_elem(map_fd, &key, &val, BPF_ANY),
+		       "fill array"))
+		goto out;
+
+	pcpu_fd = bpf_map_create(BPF_MAP_TYPE_PERCPU_ARRAY, "parr",
+				 sizeof(__u32), sizeof(__u64), max_entries, &copts);
+	if (!ASSERT_GE(pcpu_fd, 0, "percpu array map"))
+		goto out;
+
+	inner_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "inner", sizeof(__u32),
+				  sizeof(__u64), 1, &copts);
+	if (!ASSERT_GE(inner_fd, 0, "inner map"))
+		goto out;
+
+	oopts.inner_map_fd = inner_fd;
+	outer_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY_OF_MAPS, "aom",
+				  sizeof(__u32), sizeof(__u32), max_entries,
+				  &oopts);
+	if (!ASSERT_GE(outer_fd, 0, "array of maps"))
+		goto out;
+	if (!ASSERT_OK(bpf_map_update_elem(outer_fd, &key, &inner_fd, BPF_ANY),
+		       "fill array of maps"))
+		goto out;
+
+	prog_fd = load_lookup_prog(map_fd, key);
+	if (!ASSERT_GE(prog_fd, 0, "load array prog"))
+		goto out;
+	check_jmp32(prog_fd, max_entries, "array gen_lookup uses JMP32 JGE");
+	close(prog_fd);
+
+	prog_fd = load_lookup_prog(pcpu_fd, key);
+	if (!ASSERT_GE(prog_fd, 0, "load percpu prog"))
+		goto out;
+	check_jmp32(prog_fd, max_entries, "percpu gen_lookup uses JMP32 JGE");
+	close(prog_fd);
+
+	prog_fd = load_lookup_prog(outer_fd, key);
+	if (!ASSERT_GE(prog_fd, 0, "load array of maps prog"))
+		goto out;
+	check_jmp32(prog_fd, max_entries, "array of maps gen_lookup uses JMP32 JGE");
+	close(prog_fd);
+
+	/* The inlined lookup still works, in range and out of range. */
+	run_lookup(map_fd, key, true);
+	run_lookup(map_fd, max_entries + 5, false);
+	run_lookup(pcpu_fd, key, true);
+	run_lookup(pcpu_fd, max_entries + 5, false);
+	run_lookup(outer_fd, key, true);
+	run_lookup(outer_fd, max_entries + 5, false);
+
+out:
+	if (outer_fd >= 0)
+		close(outer_fd);
+	if (inner_fd >= 0)
+		close(inner_fd);
+	if (pcpu_fd >= 0)
+		close(pcpu_fd);
+	if (map_fd >= 0)
+		close(map_fd);
+}
\ No newline at end of file
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 2/2] selftests/bpf: Test the emitted bounds check of the array gen_lookup
  2026-11-15 10:25 ` [PATCH bpf-next 2/2] selftests/bpf: Test the emitted bounds check of the array gen_lookup Yuan Chen
@ 2026-11-15 10:47   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-15 10:47 UTC (permalink / raw)
  To: Yuan Chen, bpf

Thank you for your contribution! Sashiko AI review found 5 potential
issues to consider:
- [Medium] selftests/bpf: check_jmp32() fails on bpf_jit_needs_zext()
  architectures
- [Medium] selftests/bpf: verifier elides NULL check in
  load_lookup_prog() for in-range key
- [Low] selftests/bpf: test_array_gen_lookup_jmp32() fails when
  bpf_jit_harden is enabled
- [Low] selftests/bpf: check_jmp32() duplicates get_xlated_program()
  helper
- [Low] selftests/bpf: comment style and missing EOF newline in
  array_gen_lookup_jmp32.c
--

commit 10c7c22ab5fe20010aaf42d0505ab98d777d6a8e
Author: Yuan Chen <chenyuan@kylinos.cn>

selftests/bpf: Test the emitted bounds check of the array gen_lookup

This commit adds a bpf selftest that inspects the translated instructions
of inlined array map lookups for a 32-bit bounds check and verifies in-range
and out-of-range lookups via bpf_prog_test_run_opts().

> diff --git a/tools/testing/selftests/bpf/prog_tests/array_gen_lookup_jmp32.c b/tools/testing/selftests/bpf/prog_tests/array_gen_lookup_jmp32.c
> new file mode 100644
> index 0000000000000..ba8215506534a
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/array_gen_lookup_jmp32.c
> @@ -0,0 +1,204 @@
[ ... ]
> +/* Loads a program that inlines bpf_map_lookup_elem() on @map_fd and
> + * returns 2 (XDP_PASS) when the element is found, 1 (XDP_DROP) when the
> + * lookup returns NULL.
> + */

[Severity: Low]
This isn't a bug, but bpf coding style places the opening /* of a
multi-line comment on its own line, and the end of this file is missing a
trailing newline.

> +static int load_lookup_prog(int map_fd, __u32 key)
> +{
> +	const struct bpf_insn insns[] = {
> +		BPF_LD_MAP_FD(BPF_REG_1, map_fd),
> +		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> +		BPF_ST_MEM(BPF_W, BPF_REG_2, 0, key),
> +		BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
> +		BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
> +		BPF_MOV64_IMM(BPF_REG_0, 2 /* XDP_PASS */),
> +		BPF_JMP_A(1),
> +		BPF_MOV64_IMM(BPF_REG_0, 1 /* XDP_DROP */),
> +		BPF_EXIT_INSN(),
> +	};

[Severity: Medium]
Does the verifier dead-code-eliminate the null check here for in-range
array and per-cpu array keys?

Because key is stored with a constant-immediate store at an 8-byte-aligned
stack offset (r10 - 8), check_stack_write_fixed_off() tracks it as a
spilled constant scalar.  When key < max_entries, check_helper_call() sees
can_elide_value_nullness() and marks r0 as non-null:

kernel/bpf/verifier.c:check_helper_call() {
    ...
		if (func_id == BPF_FUNC_map_lookup_elem &&
		    can_elide_value_nullness(meta.map.ptr) &&
		    meta.max_map_key < meta.map.ptr->max_entries)
			ret_flag &= ~PTR_MAYBE_NULL;
    ...
}

Dead-code elimination then strips the conditional jump and the drop branch
before bpf_do_misc_fixups() inlines the lookup, so the translated program
ignores r0 from the inlined lookup and unconditionally sets r0 = 2.
Storing the key at r10 - 4 or reading the returned element pointer would
keep the runtime check intact.

[ ... ]
> +static void check_jmp32(int prog_fd, __u32 max_entries, const char *what)
> +{
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info), len;
> +	struct bpf_insn *xlated;
> +	__u32 i, jge32 = 0, jge64 = 0;
> +	int err;
> +
> +	err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (!ASSERT_OK(err, "prog_get_info"))
> +		return;
> +
> +	len = info.xlated_prog_len;
> +	xlated = calloc(1, len);
> +	if (!ASSERT_OK_PTR(xlated, "calloc xlated"))
> +		return;
> +
> +	memset(&info, 0, sizeof(info));
> +	info.xlated_prog_len = len;
> +	info.xlated_prog_insns = ptr_to_u64(xlated);
> +	err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (!ASSERT_OK(err, "prog_get_info xlated"))
> +		goto out;

[Severity: Low]
This isn't a bug, but could check_jmp32() use get_xlated_program() from
testing_helpers.h instead of open-coding the two bpf_prog_get_info_by_fd()
calls and allocation?  Also, test_array_gen_lookup_jmp32() loads the
program with key = 1 to run check_jmp32(), closes prog_fd, and then calls
run_lookup() which loads the exact same program a second time.

> +
> +	for (i = 0; i + 1 < info.xlated_prog_len / (u32)sizeof(*xlated); i++) {
> +		if (xlated[i].code != (BPF_LDX | BPF_W | BPF_MEM))
> +			continue;
> +		if (xlated[i + 1].code != (BPF_JMP32 | BPF_JGE | BPF_K) &&
> +		    xlated[i + 1].code != (BPF_JMP | BPF_JGE | BPF_K))
> +			continue;

[Severity: Medium]
Will check_jmp32() miss the bounds-check jump on 64-bit architectures where
bpf_jit_needs_zext() is true (such as s390x, riscv64, and powerpc64)?

When the patch list from map_gen_lookup() is applied, patch_list_adj_aux()
marks the 32-bit index load with zext_dst = true.  In
bpf_opt_subreg_zext_lo32_rnd_hi32(), architectures with
bpf_jit_needs_zext() insert a zero-extension instruction right after the
load at xlated[i + 1], which shifts the bounds-check jump to xlated[i + 2]
and leaves jge32 at 0.

[ ... ]
> +void test_array_gen_lookup_jmp32(void)
> +{

[Severity: Low]
Can check_jmp32() fail when /proc/sys/net/core/bpf_jit_harden is enabled
on the host or toggled by another test running in parallel under
test_progs -j?

When bpf_jit_harden is 2, bpf_jit_blind_insn() rewrites the immediate
bounds-check jump into register-based instructions using the auxiliary
register before exposing the translated instructions, so jge32 stays 0.
Other tests that inspect translated instructions (such as
serial_test_patch_list()) use the serial_test_ prefix and set
/proc/sys/net/core/bpf_jit_harden to 0 while loading the program.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115102559.465375-1-chenyuan@kylinos.cn?part=2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: Use a 32-bit compare in the array map gen_lookups
  2026-11-15 10:25 ` [PATCH bpf-next 1/2] bpf: Use a 32-bit compare in the array map gen_lookups Yuan Chen
@ 2026-11-15 11:00   ` bot+bpf-ci
  0 siblings, 0 replies; 5+ messages in thread
From: bot+bpf-ci @ 2026-11-15 11:00 UTC (permalink / raw)
  To: chenyuan, bpf
  Cc: netdev, ast, daniel, andrii, eddyz87, martin.lau, shuah,
	linux-kselftest, chenyuan, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

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

> bpf: Use a 32-bit compare in the array map gen_lookups

TL;DR:
- BPF_JMP32 may break sparc64 JIT of array and array-of-map lookups, falling
  back to interpreter or failing to load
- Commit message: incomplete bypass_spec_v1 condition and likely wrong Fixes:
  tag

> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c

[ ... ]

> @@ -230,10 +230,10 @@ static int array_map_gen_lookup(struct bpf_map *map, struct bpf_insn *insn_buf)
>  	*insn++ = BPF_ALU64_IMM(BPF_ADD, map_ptr, offsetof(struct bpf_array, value));
>  	*insn++ = BPF_LDX_MEM(BPF_W, ret, index, 0);
>  	if (!map->bypass_spec_v1) {
> -		*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 4);
> +		*insn++ = BPF_JMP32_IMM(BPF_JGE, ret, map->max_entries, 4);
>  		*insn++ = BPF_ALU32_IMM(BPF_AND, ret, array->index_mask);
>  	} else {
> -		*insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 3);
> +		*insn++ = BPF_JMP32_IMM(BPF_JGE, ret, map->max_entries, 3);
>  	}

Can this break JIT compilation on sparc64?  The inlined lookups for plain
array maps and array-of-maps now always emit a BPF_JMP32 | BPF_JGE | BPF_K
instruction, but arch/sparc/net/bpf_jit_comp_64.c has no BPF_JMP32 handling
at all.  build_insn() only has the BPF_JMP | BPF_JGE | BPF_K cases, so the
new opcode reaches:

    default:
        pr_err_once("unknown opcode %02x\n", code);
        return -EINVAL;

and bpf_int_jit_compile() returns the program un-JITed.

bpf_do_misc_fixups() inlines map_gen_lookup whenever
prog->jit_requested && BITS_PER_LONG == 64, which sparc64 satisfies with
net.core.bpf_jit_enable=1.  The old 64-bit BPF_JMP_IMM(BPF_JGE, ...) was
JITed fine there.

So every sparc64 program calling bpf_map_lookup_elem() on an array map or
array-of-maps loses its JIT, even programs with no JMP32 of their own.  In
__bpf_prog_select_runtime():

    if (!fp->jited && jit_needed) {
        *err = -ENOTSUPP;

With CONFIG_BPF_JIT_ALWAYS_ON=y these programs would then fail to load with
-ENOTSUPP, and otherwise they silently fall back to the interpreter.

There is no JIT capability gate for JMP32 here.  Could the compare avoid
JMP32, for example by loading the bound into a scratch register with a
zero-extending BPF_MOV32_IMM and using a 64-bit BPF_JMP_REG(BPF_JGE, ...)?
R1-R5 are dead after the replaced helper call.  Alternatively, should JMP32
support be added to the sparc64 JIT first?

percpu_array_map_gen_lookup() is not affected, since sparc64 returns false
from bpf_jit_supports_percpu_insn() and takes the -EOPNOTSUPP fallback.

commit 3777fda5a1d0e595d3e7d05daddd4eaed5926bc9
Author: Yuan Chen <chenyuan@kylinos.cn>

>     bpf: Use a 32-bit compare in the array map gen_lookups
>
>     array_map_gen_lookup(), percpu_array_map_gen_lookup() and
>     array_of_map_gen_lookup() bound the inlined index with
>     BPF_JMP_IMM(BPF_JGE, idx, max_entries, ...).  The index is a
>     zero-extended u32, but the immediate of a 64-bit JGE is sign-extended,
>     so for max_entries >= 0x80000000 the compare is never true.
>
>     With bypass_spec_v1, an out-of-range key can therefore produce
>     an address beyond the end of the array:
>     array->value + index * elem_size.
>     Without bypass_spec_v1 the index_mask AND keeps the access in bounds,
>     but such a key returns a masked element instead of NULL.
>
>     Creating such a map needs at least a 16 GiB allocation (max_entries
>     0x80000000, elem_size 8); the out-of-bounds path also needs a
>     CAP_PERFMON creator or mitigations=off.
>
>     Use BPF_JMP32_IMM() for the three compares, as commit 70504de0bb62
>     ("xsk: Use a 32-bit compare in xsk_map_gen_lookup") did for
>     xsk_map_gen_lookup().
>
>     Fixes: b2157399cc98 ("bpf: prevent out-of-bounds speculation")
>     Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>

The commit message says the out-of-bounds path "also needs a CAP_PERFMON
creator or mitigations=off".  Is that complete?

map->bypass_spec_v1 comes from bpf_bypass_spec_v1(NULL) in
array_map_alloc(), which is:

    return bpf_jit_bypass_spec_v1() ||
           cpu_mitigations_off() ||
           bpf_token_capable(token, CAP_PERFMON);

On LoongArch, bpf_jit_bypass_spec_v1() unconditionally returns true.  On
powerpc64 it returns true unless both SEC_FTR_FAVOUR_SECURITY and
SEC_FTR_BNDS_CHK_SPEC_BAR are set, and always on configs that are neither
E500 nor BOOK3S_64.

On those systems every array map, including one created by an unprivileged
user, takes the bypass branch, so neither CAP_PERFMON nor mitigations=off
is needed.  Could this say "or when bpf_bypass_spec_v1() is true" instead?

Also, the Fixes: tag points at b2157399cc98 ("bpf: prevent out-of-bounds
speculation"), but that commit only added the unpriv/index_mask variants of
the sequences; the 64-bit BPF_JMP_IMM(BPF_JGE, idx, max_entries) compare
already existed.  The sign-extended compare was introduced by:

  Fixes: 81ed18ab3098 ("bpf: add helper inlining infra and optimize map_array lookup")

for array_map_gen_lookup(), with the percpu and array-of-map variants coming
from db69718b8efa ("bpf: inline bpf_map_lookup_elem() for PERCPU_ARRAY
maps") and 7b0c2a0508b9 ("bpf: inline map in map lookup functions for array
and htab").  Should the Fixes: tag be updated?


---
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/43705303096

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-15 11:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-15 10:25 [PATCH bpf-next 0/2] bpf: Fix the array map gen_lookup index bounds check Yuan Chen
2026-11-15 10:25 ` [PATCH bpf-next 1/2] bpf: Use a 32-bit compare in the array map gen_lookups Yuan Chen
2026-11-15 11:00   ` bot+bpf-ci
2026-11-15 10:25 ` [PATCH bpf-next 2/2] selftests/bpf: Test the emitted bounds check of the array gen_lookup Yuan Chen
2026-11-15 10:47   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox