mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg()
@ 2026-10-18  4:21 Woojin Ji
  2026-10-18  4:21 ` [PATCH bpf v2 1/2] " Woojin Ji
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Woojin Ji @ 2026-10-18  4:21 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Shuah Khan, linux-kselftest, linux-kernel,
	Woojin Ji

v2, from the review of v1:

 - Shift the index as a 32-bit value instead of inserting an explicit
   zero-extension (Alexei). The inlined sequence is one instruction shorter
   and the bounds check is untouched, so no program is accepted or rejected
   differently.

 - The selftest is now a separate patch (Jiri). Patch 2 keeps Jiri's
   Acked-by from v1; patch 1 does not, because the kernel patch changed.

v1, the problem: bpf_get_func_arg() takes a u32 index, but the inlined
expansion compared only the low 32 bits of R2 with the argument count and
then shifted the full 64-bit register to compute the context address. R2 is
ARG_ANYTHING, so an index of 1ULL << 32 passed the bounds check as index
zero while the load was performed at ctx + (1ULL << 35).

Patch 2 depends on patch 1: without it the wide index faults in the inlined
sequence and the guest panics.

Validation, re-run with this revision (the 32-bit shift kernel):

 - Disposable x86_64 QEMU guest running a BTF-enabled kernel built with
   clang 22, with tracing and bpf_testmod. test_progs output, verbatim:

     #150     get_func_args_fsession_test:OK
     #151     get_func_args_test:OK
     Summary: 2/0 PASSED, 0 SKIPPED, 0/0 FAILED

   The pairs in that line are top-level tests/subtests, so both tests pass
   with nothing skipped or failed; TEST_PROGS_EXIT=0 and the guest dmesg is
   clean.
 - The same two tests on unpatched ff47652a4b66 fault in the inlined
   sequence: a page fault in bpf_prog_..._test2, with the load performed at
   ctx + (1ULL << 35), which panics the guest.
 - bpftool xlated dumps of both test programs show the index shifted as a
   32-bit value (w2 <<= 3) at all 32 inlined helper sites on this kernel,
   and the 64-bit shift (r2 <<= 3) at all 32 on unpatched ff47652a4b66.

v1: https://lore.kernel.org/bpf/179221432098.1890560.17043109037413683447.get-func-arg-index@gmail.com/

---
Woojin Ji (2):
      bpf: Zero-extend the index in inlined bpf_get_func_arg()
      selftests/bpf: Test out-of-range index bits in bpf_get_func_arg()

 kernel/bpf/fixups.c                                |  2 +-
 .../selftests/bpf/progs/get_func_args_test.c       | 36 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)
---
base-commit: ff47652a4b66c067c765a7ad464d930b5a9367cc
change-id: 20261018-get-func-arg-93c80d63ef45

Best regards,
-- 
Woojin Ji <random6.xyz@gmail.com>


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

* [PATCH bpf v2 1/2] bpf: Zero-extend the index in inlined bpf_get_func_arg()
  2026-10-18  4:21 [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg() Woojin Ji
@ 2026-10-18  4:21 ` Woojin Ji
  2026-10-18  4:55   ` bot+bpf-ci
  2026-10-18  4:21 ` [PATCH bpf v2 2/2] selftests/bpf: Test out-of-range index bits in bpf_get_func_arg() Woojin Ji
  2026-10-18 15:30 ` [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg() patchwork-bot+netdevbpf
  2 siblings, 1 reply; 5+ messages in thread
From: Woojin Ji @ 2026-10-18  4:21 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Shuah Khan, linux-kselftest, linux-kernel,
	Woojin Ji

bpf_get_func_arg() takes a u32 index, but the inlined expansion compared
only the low 32 bits of R2 with the argument count and then shifted the
full 64-bit register to compute the context address. R2 is ARG_ANYTHING,
so nothing guarantees that its upper bits are zero: an index of
1ULL << 32 passes the bounds check as index zero while the load is
performed at ctx + (1ULL << 35).

Shift the index as a 32-bit value. When the 32-bit bounds check passes,
the low 32 bits of R2 are below R0, and R0 is at most 255: the trampoline
arm masks the argument count with 0xFF and the raw_tp arm uses the BTF
argument count minus one, which the verifier caps at MAX_BPF_FUNC_ARGS.
The 3-bit shift therefore cannot overflow and it clears the upper bits of
the register. Widening the comparison instead would reject indices whose
low 32 bits are valid. The check itself is unchanged, so this cannot
change which programs are accepted, only the address that is computed.

Fixes: f92c1e183604 ("bpf: Add get_func_[arg|ret|arg_cnt] helpers")
Assisted-by: LLM
Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
---
 kernel/bpf/fixups.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index d6f83521fc78e6c6a82d1969662f1eb52f64f4c3..ec62df6811e0b0b9da11405cf189c0f0419a4496 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -2256,7 +2256,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
 				cnt = 2;
 			}
 			insn_buf[cnt++] = BPF_JMP32_REG(BPF_JGE, BPF_REG_2, BPF_REG_0, 6);
-			insn_buf[cnt++] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 3);
+			insn_buf[cnt++] = BPF_ALU32_IMM(BPF_LSH, BPF_REG_2, 3);
 			insn_buf[cnt++] = BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_1);
 			insn_buf[cnt++] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, 0);
 			insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_3, BPF_REG_0, 0);

-- 
2.47.3


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

* [PATCH bpf v2 2/2] selftests/bpf: Test out-of-range index bits in bpf_get_func_arg()
  2026-10-18  4:21 [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg() Woojin Ji
  2026-10-18  4:21 ` [PATCH bpf v2 1/2] " Woojin Ji
@ 2026-10-18  4:21 ` Woojin Ji
  2026-10-18 15:30 ` [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg() patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Woojin Ji @ 2026-10-18  4:21 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Shuah Khan, linux-kselftest, linux-kernel,
	Woojin Ji

The inlined expansion of bpf_get_func_arg() bounds-checks only the low 32
bits of the index, so a helper prototype that keeps the upper bits of the
index is needed to reach it.

Add check_wide_index() and call it from the fexit and tp_btf tests. It
drives the helper with indices that have upper bits set and with indices
whose low 32 bits are out of range, and it checks that the destination is
left untouched when the helper reports -EINVAL. Without the previous patch
the wide index is shifted as 64-bit and the load uses an address far
outside the context.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Assisted-by: LLM
Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
---
 .../selftests/bpf/progs/get_func_args_test.c       | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/get_func_args_test.c b/tools/testing/selftests/bpf/progs/get_func_args_test.c
index 1bf47f64d0967a3543739841201b58b14b4036a7..9b0beb58d88d5bcf1a36a0598695fd02a2cebbcf 100644
--- a/tools/testing/selftests/bpf/progs/get_func_args_test.c
+++ b/tools/testing/selftests/bpf/progs/get_func_args_test.c
@@ -6,6 +6,40 @@
 
 char _license[] SEC("license") = "GPL";
 
+/* Keep the upper index bits: the inlined expansion must truncate them. */
+static long (*bpf_get_func_arg_wide)(void *ctx, __u64 n, __u64 *value) =
+	(void *)BPF_FUNC_get_func_arg;
+
+/* Both traced functions are called with arguments 2 and 3. */
+static __always_inline int check_wide_index(void *ctx)
+{
+	__u64 a = 0, z = 0xdeadbeefdeadbeefULL;
+	__s64 err;
+	int ok = 1;
+
+	/* The index is truncated to u32, so these select arguments 0 and 1. */
+	err = bpf_get_func_arg_wide(ctx, 1ULL << 32, &a);
+	ok &= err == 0 && ((int)a == 2);
+	err = bpf_get_func_arg_wide(ctx, (1ULL << 32) | 1, &a);
+	ok &= err == 0 && a == 3;
+	err = bpf_get_func_arg_wide(ctx, 1ULL << 59, &a);
+	ok &= err == 0 && ((int)a == 2);
+	err = bpf_get_func_arg_wide(ctx, (1ULL << 59) | 1, &a);
+	ok &= err == 0 && a == 3;
+
+	/* Out of range: -EINVAL and no store into the destination. */
+	err = bpf_get_func_arg_wide(ctx, (1ULL << 32) | 2, &z);
+	ok &= err == -EINVAL && z == 0xdeadbeefdeadbeefULL;
+	err = bpf_get_func_arg_wide(ctx, (1ULL << 32) | 0xffffffffULL, &z);
+	ok &= err == -EINVAL && z == 0xdeadbeefdeadbeefULL;
+	err = bpf_get_func_arg_wide(ctx, (1ULL << 59) | 2, &z);
+	ok &= err == -EINVAL && z == 0xdeadbeefdeadbeefULL;
+	err = bpf_get_func_arg_wide(ctx, (1ULL << 59) | 0xffffffffULL, &z);
+	ok &= err == -EINVAL && z == 0xdeadbeefdeadbeefULL;
+
+	return ok;
+}
+
 __u64 test1_result = 0;
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG(test1)
@@ -63,6 +97,7 @@ int BPF_PROG(test2)
 	/* return value */
 	err = bpf_get_func_ret(ctx, &ret);
 	test2_result &= err == 0 && ret == 5;
+	test2_result &= check_wide_index(ctx);
 	return 0;
 }
 
@@ -162,6 +197,7 @@ int BPF_PROG(tp_test2)
 	/* not valid argument */
 	err = bpf_get_func_arg(ctx, 2, &z);
 	test6_result &= err == -EINVAL;
+	test6_result &= check_wide_index(ctx);
 
 	return 0;
 }

-- 
2.47.3


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

* Re: [PATCH bpf v2 1/2] bpf: Zero-extend the index in inlined bpf_get_func_arg()
  2026-10-18  4:21 ` [PATCH bpf v2 1/2] " Woojin Ji
@ 2026-10-18  4:55   ` bot+bpf-ci
  0 siblings, 0 replies; 5+ messages in thread
From: bot+bpf-ci @ 2026-10-18  4:55 UTC (permalink / raw)
  To: random6.xyz, bpf
  Cc: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai, shuah,
	linux-kselftest, linux-kernel, random6.xyz, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai

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

> bpf: Zero-extend the index in inlined bpf_get_func_arg()

TL;DR: Commit message only: the code looks correct, but the claim that the
raw_tp argument count is capped at MAX_BPF_FUNC_ARGS appears inaccurate; the
real bound is the 24-bit BTF vlen.

>     bpf: Zero-extend the index in inlined bpf_get_func_arg()
>
>     bpf_get_func_arg() takes a u32 index, but the inlined expansion compared
>     only the low 32 bits of R2 with the argument count and then shifted the
>     full 64-bit register to compute the context address. R2 is ARG_ANYTHING,
>     so nothing guarantees that its upper bits are zero: an index of
>     1ULL << 32 passes the bounds check as index zero while the load is
>     performed at ctx + (1ULL << 35).
>
>     Shift the index as a 32-bit value. When the 32-bit bounds check passes,
>     the low 32 bits of R2 are below R0, and R0 is at most 255: the trampoline
>     arm masks the argument count with 0xFF and the raw_tp arm uses the BTF
>     argument count minus one, which the verifier caps at MAX_BPF_FUNC_ARGS.
>     The 3-bit shift therefore cannot overflow and it clears the upper bits of
>     the register. Widening the comparison instead would reject indices whose
>     low 32 bits are valid. The check itself is unchanged, so this cannot
>     change which programs are accepted, only the address that is computed.
>
>     Fixes: f92c1e183604 ("bpf: Add get_func_[arg|ret|arg_cnt] helpers")
>     Assisted-by: LLM
>     Signed-off-by: Woojin Ji <random6.xyz@gmail.com>

This isn't a bug in the code, but is the commit message accurate about the
raw_tp arm?  It says R0 is at most 255 because the raw_tp arm uses "the BTF
argument count minus one, which the verifier caps at MAX_BPF_FUNC_ARGS".

For BPF_TRACE_RAW_TP, bpf_check_attach_target() only checks that the
typedef's target is a func_proto and then leaves the switch.  Unlike the
fentry/fexit/iter arms, it never calls btf_distill_func_proto(), which is
the only place nargs > MAX_BPF_FUNC_ARGS is rejected.

btf_ctx_access() only does:

    nr_args = btf_type_vlen(t);
    if (attach_btf_trace)
        nr_args--;

and compares the ctx argument index against it.  The MAX_BPF_FUNC_ARGS limit
that does exist for tracing programs is in bpf_tracing_ctx_access(), which
bounds ctx load offsets rather than the value that
BPF_MOV64_IMM(BPF_REG_0, nr_args - 1) places in R0.

The generated code itself looks correct, since btf_type_vlen() is a 24-bit
field (BTF_MAX_VLEN is 0x00ffffff), so (nr_args - 1) << 3 stays below 2^27
and cannot overflow a 32-bit shift.

Could the changelog either drop the "caps at MAX_BPF_FUNC_ARGS" claim for the
raw_tp arm, or state the real bound (the BTF vlen is 24 bits, so the shifted
value always fits in 32 bits)?


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

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

* Re: [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg()
  2026-10-18  4:21 [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg() Woojin Ji
  2026-10-18  4:21 ` [PATCH bpf v2 1/2] " Woojin Ji
  2026-10-18  4:21 ` [PATCH bpf v2 2/2] selftests/bpf: Test out-of-range index bits in bpf_get_func_arg() Woojin Ji
@ 2026-10-18 15:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-18 15:30 UTC (permalink / raw)
  To: Woojin Ji
  Cc: bpf, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai, shuah,
	linux-kselftest, linux-kernel

Hello:

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

On Sun, 18 Oct 2026 13:21:40 +0900 you wrote:
> v2, from the review of v1:
> 
>  - Shift the index as a 32-bit value instead of inserting an explicit
>    zero-extension (Alexei). The inlined sequence is one instruction shorter
>    and the bounds check is untouched, so no program is accepted or rejected
>    differently.
> 
> [...]

Here is the summary with links:
  - [bpf,v2,1/2] bpf: Zero-extend the index in inlined bpf_get_func_arg()
    https://git.kernel.org/bpf/bpf-next/c/1c1dda9ba9f3
  - [bpf,v2,2/2] selftests/bpf: Test out-of-range index bits in bpf_get_func_arg()
    https://git.kernel.org/bpf/bpf-next/c/38dbc3047a2f

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



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

end of thread, other threads:[~2026-10-18 15:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-18  4:21 [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg() Woojin Ji
2026-10-18  4:21 ` [PATCH bpf v2 1/2] " Woojin Ji
2026-10-18  4:55   ` bot+bpf-ci
2026-10-18  4:21 ` [PATCH bpf v2 2/2] selftests/bpf: Test out-of-range index bits in bpf_get_func_arg() Woojin Ji
2026-10-18 15:30 ` [PATCH bpf v2 0/2] bpf: Zero-extend the index in inlined bpf_get_func_arg() patchwork-bot+netdevbpf

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

all inboxes | Powered by JetHome®