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

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

[Woojin Ji]

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(+)

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

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

[Woojin Ji]

The initial posting of this patch went out without a Cc list; adding it
here. Nothing in the patch or in its commit message changed since that
posting, so there is nothing to re-review:

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

For reference, the change under review is:

 kernel/bpf/fixups.c                                |  5 +++
 .../selftests/bpf/progs/get_func_args_test.c       | 36 ++++++++++++++++++++++
 2 files changed, 41 insertions(+)

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

[Jiri Olsa]

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
> 

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

[Alexei Starovoitov]

On Sat, Oct 17, 2026 at 02:42 PM Woojin Ji <random6.xyz@gmail.com> wrote:
> +			/*
> +			 * 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);

After JMP32 the low 32 bits of R2 are below nr_args, which is at most
255, so a 32-bit shift cannot overflow and it clears the upper bits.
can we just make the shift 32-bit ?
Then no need for the extra insn and the comment.
The selftest stays as is.
Something like this (compile tested only):

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index d6f8352..ec62df6 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -2256,7 +2256,7 @@ patch_map_ops_generic:
 				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);

pw-bot: cr