Re: [PATCH bpf] bpf: Zero-extend the index in inlined bpf_get_func_arg()

[Jiri Olsa <olsajiri@gmail.com>]

On Sat, Oct 17, 2026 at 02:42:24PM +0900, Woojin Ji wrote:
> bpf_get_func_arg() takes a u32 index, but the inlined expansion compares
> only the low 32 bits of R2 with the argument count and then shifts 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).
> 
> Zero-extend R2 to the u32 index the helper takes, which is what the C
> implementation does; that path is still reachable because LSM programs
> with BPF_LSM_MAC are not inlined. Widening the comparison instead would
> reject indices whose low 32 bits are valid. Since the check is 32-bit,
> this cannot change which programs are accepted, only the address.
> 
> Extend the existing fexit and tp_btf tests with a 64-bit helper
> prototype so the upper index bits reach the expansion.
> 
> Assisted-by: LLM
> Fixes: f92c1e183604 ("bpf: Add get_func_[arg|ret|arg_cnt] helpers")
> Signed-off-by: Woojin Ji <random6.xyz@gmail.com>
> ---
> Validation:
>   - Built the modified objects and both affected test programs.
>   - Disposable x86_64 QEMU guest, clang-22 kernel with BTF, tracing and
>     bpf_testmod: both tests pass (2/0 PASSED, 0 SKIPPED, 0 FAILED) and
>     the guest dmesg is clean.
>   - Same tests on unpatched ff47652a4b66: the wide index faults in the
>     inline sequence and the guest panics.
> 
>  kernel/bpf/fixups.c                                |  5 +++
>  .../selftests/bpf/progs/get_func_args_test.c       | 36 ++++++++++++++++++++++
>  2 files changed, 41 insertions(+)

nice, please split kernel and test into separate patches

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka


> 
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index d6f83521fc78e6c6a82d1969662f1eb52f64f4c3..005ed9279ebab60a8a774c4e6937232ab4f38962 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -2255,6 +2255,11 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
>  				insn_buf[1] = BPF_ALU64_IMM(BPF_AND, BPF_REG_0, 0xFF);
>  				cnt = 2;
>  			}
> +			/*
> +			 * The helper takes a u32 index; zero-extend it before
> +			 * the bounds check and the 64-bit address math below.
> +			 */
> +			insn_buf[cnt++] = BPF_MOV32_REG(BPF_REG_2, BPF_REG_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_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_1);
> 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;
>  }
> 
> base-commit: ff47652a4b66c067c765a7ad464d930b5a9367cc
> -- 
> 2.47.3
>