> 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 <[email protected]>
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
