[PATCH bpf-next 0/3] properly load insn array values with offsets

[PATCH bpf-next 0/3] properly load insn array values with offsets

[Anton Protopopov]

As was reported by the BPF CI bot in [1] the direct address
of an instruction array returned by map_direct_value_addr()
is incorrect if the offset is non-zero. Fix this bug and
add selftests.

Also (commit 2), return EACCES instead of EINVAL when offsets
aren't correct.

  [1] https://lore.kernel.org/bpf/0447c47ac58306546a5dbdbad2601f3e77fa8eb24f3a4254dda3a39f6133e68f@mail.kernel.org/

Anton Protopopov (3):
  bpf: insn array: return proper address for non-zero offsets
  bpf: insn array: return EACCES for incorrect map access
  selftests/bpf: add tests for loading map values with offsets

 kernel/bpf/bpf_insn_array.c                   |   4 +-
 .../selftests/bpf/prog_tests/bpf_gotox.c      | 208 ++++++++++++++++++
 2 files changed, 210 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH bpf-next 1/3] bpf: insn array: return proper address for non-zero offsets

[Anton Protopopov]

The map_direct_value_addr() function of the instruction
array map incorrectly adds offset to the resulting address.
This is a bug, because later the resolve_pseudo_ldimm64()
function adds the offset. Fix it. Corresponding selftests
are added in a consequent commit.

Fixes: 493d9e0d6083 ("bpf, x86: add support for indirect jumps")
Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
---
 kernel/bpf/bpf_insn_array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c
index c96630cb75bf..37b43102953e 100644
--- a/kernel/bpf/bpf_insn_array.c
+++ b/kernel/bpf/bpf_insn_array.c
@@ -126,7 +126,7 @@ static int insn_array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
 		return -EINVAL;
 
 	/* from BPF's point of view, this map is a jump table */
-	*imm = (unsigned long)insn_array->ips + off;
+	*imm = (unsigned long)insn_array->ips;
 
 	return 0;
 }
-- 
2.34.1

[PATCH bpf-next 2/3] bpf: insn array: return EACCES for incorrect map access

[Anton Protopopov]

The insn_array_map_direct_value_addr() function currently returns
-EINVAL when the offset within the map is invalid. Change this to
return -EACCES, so that it is consistent with similar boundary access
checks in the verifier.

Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
---
 kernel/bpf/bpf_insn_array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c
index 37b43102953e..c0286f25ca3c 100644
--- a/kernel/bpf/bpf_insn_array.c
+++ b/kernel/bpf/bpf_insn_array.c
@@ -123,7 +123,7 @@ static int insn_array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
 
 	if ((off % sizeof(long)) != 0 ||
 	    (off / sizeof(long)) >= map->max_entries)
-		return -EINVAL;
+		return -EACCES;
 
 	/* from BPF's point of view, this map is a jump table */
 	*imm = (unsigned long)insn_array->ips;
-- 
2.34.1

[PATCH bpf-next 3/3] selftests/bpf: add tests for loading map values with offsets

[Anton Protopopov]

The ldimm64 instruction for map value supports an offset.
For insn array maps it wasn't tested before, as normally
such instructions aren't generated. However, this is still
possible to pass such instructions, so add a few tests to
check that correct offsets work properly and incorrect
offsets are rejected.

Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
---
 .../selftests/bpf/prog_tests/bpf_gotox.c      | 208 ++++++++++++++++++
 1 file changed, 208 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
index d138cc7b1bda..75b0cf2467ab 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
@@ -240,6 +240,208 @@ static void check_nonstatic_global_other_sec(struct bpf_gotox *skel)
 	bpf_link__destroy(link);
 }
 
+/*
+ * The following subtests do not use skeleton rather than to check
+ * if the test should be skipped.
+ */
+
+static int create_jt_map(__u32 max_entries)
+{
+	const char *map_name = "jt";
+	__u32 key_size = 4;
+	__u32 value_size = sizeof(struct bpf_insn_array_value);
+
+	return bpf_map_create(BPF_MAP_TYPE_INSN_ARRAY, map_name,
+			      key_size, value_size, max_entries, NULL);
+}
+
+static int prog_load(struct bpf_insn *insns, __u32 insn_cnt)
+{
+	return bpf_prog_load(BPF_PROG_TYPE_RAW_TRACEPOINT, NULL, "GPL", insns, insn_cnt, NULL);
+}
+
+static int __check_ldimm64_off_prog_load(__u32 max_entries, __u32 off)
+{
+	struct bpf_insn insns[] = {
+		BPF_LD_IMM64_RAW(BPF_REG_1, BPF_PSEUDO_MAP_VALUE, 0),
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	};
+	int map_fd, ret;
+
+	map_fd = create_jt_map(max_entries);
+	if (!ASSERT_GE(map_fd, 0, "create_jt_map"))
+		return -1;
+	if (!ASSERT_EQ(bpf_map_freeze(map_fd), 0, "bpf_map_freeze")) {
+		close(map_fd);
+		return -1;
+	}
+
+	insns[0].imm = map_fd;
+	insns[1].imm = off;
+
+	ret = prog_load(insns, ARRAY_SIZE(insns));
+	close(map_fd);
+	return ret;
+}
+
+/*
+ * Check that loads from an instruction array map are only allowed with offsets
+ * which are multiples of 8 and do not point to outside of the map.
+ */
+static void check_ldimm64_off_load(struct bpf_gotox *skel __always_unused)
+{
+	const __u32 max_entries = 10;
+	int prog_fd;
+	__u32 off;
+
+	for (off = 0; off < max_entries; off++) {
+		prog_fd = __check_ldimm64_off_prog_load(max_entries, off * 8);
+		if (!ASSERT_GE(prog_fd, 0, "__check_ldimm64_off_prog_load"))
+			return;
+		close(prog_fd);
+	}
+
+	prog_fd = __check_ldimm64_off_prog_load(max_entries, 7 /* not a multiple of 8 */);
+	if (!ASSERT_EQ(prog_fd, -EACCES, "__check_ldimm64_off_prog_load: should be -EACCES")) {
+		close(prog_fd);
+		return;
+	}
+
+	prog_fd = __check_ldimm64_off_prog_load(max_entries, max_entries * 8 /* too large */);
+	if (!ASSERT_EQ(prog_fd, -EACCES, "__check_ldimm64_off_prog_load: should be -EACCES")) {
+		close(prog_fd);
+		return;
+	}
+}
+
+static int __check_ldimm64_gotox_prog_load(struct bpf_insn *insns,
+					   __u32 insn_cnt,
+					   __u32 off1, __u32 off2)
+{
+	const __u32 values[] = {5, 7, 9, 11, 13, 15};
+	const __u32 max_entries = ARRAY_SIZE(values);
+	struct bpf_insn_array_value val = {};
+	int map_fd, ret, i;
+
+	map_fd = create_jt_map(max_entries);
+	if (!ASSERT_GE(map_fd, 0, "create_jt_map"))
+		return -1;
+
+	for (i = 0; i < max_entries; i++) {
+		val.orig_off = values[i];
+		if (!ASSERT_EQ(bpf_map_update_elem(map_fd, &i, &val, 0), 0,
+			       "bpf_map_update_elem")) {
+			close(map_fd);
+			return -1;
+		}
+	}
+
+	if (!ASSERT_EQ(bpf_map_freeze(map_fd), 0, "bpf_map_freeze")) {
+		close(map_fd);
+		return -1;
+	}
+
+	/* r1 = &map + offset1 */
+	insns[0].imm = map_fd;
+	insns[1].imm = off1;
+
+	/* r1 += off2 */
+	insns[2].imm = off2;
+
+	ret = prog_load(insns, insn_cnt);
+	close(map_fd);
+	return ret;
+}
+
+static void reject_offsets(struct bpf_insn *insns, __u32 insn_cnt, __u32 off1, __u32 off2)
+{
+	int prog_fd;
+
+	prog_fd = __check_ldimm64_gotox_prog_load(insns, insn_cnt, off1, off2);
+	if (!ASSERT_EQ(prog_fd, -EACCES, "__check_ldimm64_gotox_prog_load"))
+		close(prog_fd);
+}
+
+/*
+ * Verify a bit more complex programs which include indirect jumps
+ * and with jump tables loaded with a non-zero offset
+ */
+static void check_ldimm64_off_gotox(struct bpf_gotox *skel __always_unused)
+{
+	struct bpf_insn insns[] = {
+		/*
+		 * The following instructions perform an indirect jump to
+		 * labels below. Thus valid offsets in the map are {0,...,5}.
+		 * The program rewrites the offsets in the instructions below:
+		 *     r1 = &map + offset1
+		 *     r1 += offset2
+		 *     r1 = *r1
+		 *     gotox r1
+		 */
+		BPF_LD_IMM64_RAW(BPF_REG_1, BPF_PSEUDO_MAP_VALUE, 0),
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 0),
+		BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0),
+		BPF_RAW_INSN(BPF_JMP | BPF_JA | BPF_X, BPF_REG_1, 0, 0, 0),
+
+		/* case 0: */
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+		/* case 1: */
+		BPF_MOV64_IMM(BPF_REG_0, 1),
+		BPF_EXIT_INSN(),
+		/* case 2: */
+		BPF_MOV64_IMM(BPF_REG_0, 2),
+		BPF_EXIT_INSN(),
+		/* case 3: */
+		BPF_MOV64_IMM(BPF_REG_0, 3),
+		BPF_EXIT_INSN(),
+		/* case 4: */
+		BPF_MOV64_IMM(BPF_REG_0, 4),
+		BPF_EXIT_INSN(),
+		/* default: */
+		BPF_MOV64_IMM(BPF_REG_0, 5),
+		BPF_EXIT_INSN(),
+	};
+	int prog_fd, err;
+	__u32 off1, off2;
+
+	/* allow all combinations off1 + off2 < 6 */
+	for (off1 = 0; off1 < 6; off1++) {
+		for (off2 = 0; off1 + off2 < 6; off2++) {
+			LIBBPF_OPTS(bpf_test_run_opts, topts);
+
+			prog_fd = __check_ldimm64_gotox_prog_load(insns, ARRAY_SIZE(insns),
+								  off1 * 8, off2 * 8);
+			if (!ASSERT_GE(prog_fd, 0, "__check_ldimm64_gotox_prog_load"))
+				return;
+
+			err = bpf_prog_test_run_opts(prog_fd, &topts);
+			if (!ASSERT_OK(err, "test_run_opts err")) {
+				close(prog_fd);
+				return;
+			}
+
+			if (!ASSERT_EQ(topts.retval, off1 + off2, "test_run_opts retval")) {
+				close(prog_fd);
+				return;
+			}
+
+			close(prog_fd);
+		}
+	}
+
+	/* reject off1 + off2 >= 6 */
+	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 8 * 3);
+	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 7, 8 * 0);
+	reject_offsets(insns, ARRAY_SIZE(insns), 8 * 0, 8 * 7);
+
+	/* reject (off1 + off2) % 8 != 0 */
+	reject_offsets(insns, ARRAY_SIZE(insns), 3, 3);
+	reject_offsets(insns, ARRAY_SIZE(insns), 7, 0);
+	reject_offsets(insns, ARRAY_SIZE(insns), 0, 7);
+}
+
 void test_bpf_gotox(void)
 {
 	struct bpf_gotox *skel;
@@ -288,5 +490,11 @@ void test_bpf_gotox(void)
 	if (test__start_subtest("one-map-two-jumps"))
 		__subtest(skel, check_one_map_two_jumps);
 
+	if (test__start_subtest("check-ldimm64-off"))
+		__subtest(skel, check_ldimm64_off_load);
+
+	if (test__start_subtest("check-ldimm64-off-gotox"))
+		__subtest(skel, check_ldimm64_off_gotox);
+
 	bpf_gotox__destroy(skel);
 }
-- 
2.34.1

Re: [PATCH bpf-next 1/3] bpf: insn array: return proper address for non-zero offsets

[Emil Tsalapatis]

On Sun Jan 11, 2026 at 10:30 AM EST, Anton Protopopov wrote:
> The map_direct_value_addr() function of the instruction
> array map incorrectly adds offset to the resulting address.
> This is a bug, because later the resolve_pseudo_ldimm64()
> function adds the offset. Fix it. Corresponding selftests
> are added in a consequent commit.
>
> Fixes: 493d9e0d6083 ("bpf, x86: add support for indirect jumps")
> Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

> ---
>  kernel/bpf/bpf_insn_array.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c
> index c96630cb75bf..37b43102953e 100644
> --- a/kernel/bpf/bpf_insn_array.c
> +++ b/kernel/bpf/bpf_insn_array.c
> @@ -126,7 +126,7 @@ static int insn_array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
>  		return -EINVAL;
>  
>  	/* from BPF's point of view, this map is a jump table */
> -	*imm = (unsigned long)insn_array->ips + off;
> +	*imm = (unsigned long)insn_array->ips;
>  
>  	return 0;
>  }

Re: [PATCH bpf-next 2/3] bpf: insn array: return EACCES for incorrect map access

[Emil Tsalapatis]

On Sun Jan 11, 2026 at 10:30 AM EST, Anton Protopopov wrote:
> The insn_array_map_direct_value_addr() function currently returns
> -EINVAL when the offset within the map is invalid. Change this to
> return -EACCES, so that it is consistent with similar boundary access
> checks in the verifier.
>
> Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> ---
>  kernel/bpf/bpf_insn_array.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c
> index 37b43102953e..c0286f25ca3c 100644
> --- a/kernel/bpf/bpf_insn_array.c
> +++ b/kernel/bpf/bpf_insn_array.c
> @@ -123,7 +123,7 @@ static int insn_array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
>  
>  	if ((off % sizeof(long)) != 0 ||
>  	    (off / sizeof(long)) >= map->max_entries)
> -		return -EINVAL;
> +		return -EACCES;

-EACCES is reasonable but the other .direct_valud_addr() methods use
-EINVAL (array) and -ERANGE (arena). If we're going for consistency
can we change them all to the same error code?

>  
>  	/* from BPF's point of view, this map is a jump table */
>  	*imm = (unsigned long)insn_array->ips;

Re: [PATCH bpf-next 2/3] bpf: insn array: return EACCES for incorrect map access

[Anton Protopopov]

On Mon, Jan 12, 2026 at 6:12 PM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> On Sun Jan 11, 2026 at 10:30 AM EST, Anton Protopopov wrote:
> > The insn_array_map_direct_value_addr() function currently returns
> > -EINVAL when the offset within the map is invalid. Change this to
> > return -EACCES, so that it is consistent with similar boundary access
> > checks in the verifier.
> >
> > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > ---
> >  kernel/bpf/bpf_insn_array.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c
> > index 37b43102953e..c0286f25ca3c 100644
> > --- a/kernel/bpf/bpf_insn_array.c
> > +++ b/kernel/bpf/bpf_insn_array.c
> > @@ -123,7 +123,7 @@ static int insn_array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
> >
> >       if ((off % sizeof(long)) != 0 ||
> >           (off / sizeof(long)) >= map->max_entries)
> > -             return -EINVAL;
> > +             return -EACCES;
>
> -EACCES is reasonable but the other .direct_valud_addr() methods use
> -EINVAL (array) and -ERANGE (arena). If we're going for consistency
> can we change them all to the same error code?

Would be nice, but I doubt this is possible, as this should break
plenty of existing user space progs (for insn array userspace code
with off != 0 actually don't exist afaik).

(I am also pretty fine with not doing s/EINVAL/EACCESS for insn array,
but without this change selftests in the third patch look kinda
weird.)

> >
> >       /* from BPF's point of view, this map is a jump table */
> >       *imm = (unsigned long)insn_array->ips;
>

Re: [PATCH bpf-next 2/3] bpf: insn array: return EACCES for incorrect map access

[Emil Tsalapatis]

On Mon Jan 12, 2026 at 1:45 PM EST, Anton Protopopov wrote:
> On Mon, Jan 12, 2026 at 6:12 PM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>>
>> On Sun Jan 11, 2026 at 10:30 AM EST, Anton Protopopov wrote:
>> > The insn_array_map_direct_value_addr() function currently returns
>> > -EINVAL when the offset within the map is invalid. Change this to
>> > return -EACCES, so that it is consistent with similar boundary access
>> > checks in the verifier.
>> >
>> > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
>> > ---
>> >  kernel/bpf/bpf_insn_array.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c
>> > index 37b43102953e..c0286f25ca3c 100644
>> > --- a/kernel/bpf/bpf_insn_array.c
>> > +++ b/kernel/bpf/bpf_insn_array.c
>> > @@ -123,7 +123,7 @@ static int insn_array_map_direct_value_addr(const struct bpf_map *map, u64 *imm,
>> >
>> >       if ((off % sizeof(long)) != 0 ||
>> >           (off / sizeof(long)) >= map->max_entries)
>> > -             return -EINVAL;
>> > +             return -EACCES;
>>
>> -EACCES is reasonable but the other .direct_valud_addr() methods use
>> -EINVAL (array) and -ERANGE (arena). If we're going for consistency
>> can we change them all to the same error code?
>
> Would be nice, but I doubt this is possible, as this should break
> plenty of existing user space progs (for insn array userspace code
> with off != 0 actually don't exist afaik).
>

Makes sense.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

> (I am also pretty fine with not doing s/EINVAL/EACCESS for insn array,
> but without this change selftests in the third patch look kinda
> weird.)
>
>> >
>> >       /* from BPF's point of view, this map is a jump table */
>> >       *imm = (unsigned long)insn_array->ips;
>>

Re: [PATCH bpf-next 0/3] properly load insn array values with offsets

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Sun, 11 Jan 2026 15:30:44 +0000 you wrote:
> As was reported by the BPF CI bot in [1] the direct address
> of an instruction array returned by map_direct_value_addr()
> is incorrect if the offset is non-zero. Fix this bug and
> add selftests.
> 
> Also (commit 2), return EACCES instead of EINVAL when offsets
> aren't correct.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/3] bpf: insn array: return proper address for non-zero offsets
    https://git.kernel.org/bpf/bpf-next/c/e3bd7bdf5ffe
  - [bpf-next,2/3] bpf: insn array: return EACCES for incorrect map access
    https://git.kernel.org/bpf/bpf-next/c/7e525860e725
  - [bpf-next,3/3] selftests/bpf: add tests for loading map values with offsets
    https://git.kernel.org/bpf/bpf-next/c/c656807675e0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf-next 1/3] bpf: insn array: return proper address for non-zero offsets

[Alexei Starovoitov]

On Sun, Jan 11, 2026 at 7:23 AM Anton Protopopov
<a.s.protopopov@gmail.com> wrote:
>
> The map_direct_value_addr() function of the instruction
> array map incorrectly adds offset to the resulting address.
> This is a bug, because later the resolve_pseudo_ldimm64()
> function adds the offset. Fix it. Corresponding selftests
> are added in a consequent commit.
>
> Fixes: 493d9e0d6083 ("bpf, x86: add support for indirect jumps")
> Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>

Applied and tweaked the subject line of all 3 patches.
Please see what I did and don't invent new prefixes.
bpf, libbpf, selftest/bpf, bpftool, and "bpf, x86/arm64:"
are the only categories.
If you see others, maintainers were too lazy to do fixups.

Re: [PATCH bpf-next 1/3] bpf: insn array: return proper address for non-zero offsets

[Anton Protopopov]

On 26/01/13 07:42PM, Alexei Starovoitov wrote:
> On Sun, Jan 11, 2026 at 7:23 AM Anton Protopopov
> <a.s.protopopov@gmail.com> wrote:
> >
> > The map_direct_value_addr() function of the instruction
> > array map incorrectly adds offset to the resulting address.
> > This is a bug, because later the resolve_pseudo_ldimm64()
> > function adds the offset. Fix it. Corresponding selftests
> > are added in a consequent commit.
> >
> > Fixes: 493d9e0d6083 ("bpf, x86: add support for indirect jumps")
> > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> 
> Applied and tweaked the subject line of all 3 patches.
> Please see what I did and don't invent new prefixes.
> bpf, libbpf, selftest/bpf, bpftool, and "bpf, x86/arm64:"
> are the only categories.
> If you see others, maintainers were too lazy to do fixups.

Got it, thanks!