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

[bot+bpf-ci@kernel.org]

[-- 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