On Fri, May 2, 2025 at 7:26 AM Leon Hwang <leon.hw...@linux.dev> wrote:
>
>
>
> On 2025/5/1 00:53, Alexei Starovoitov wrote:
> > On Wed, Apr 30, 2025 at 8:55 AM Leon Hwang <leon.hw...@linux.dev> wrote:
> >>
> >>
> >>
> >> On 2025/4/30 20:43, Kafai Wan wrote:
> >>> On Wed, Apr 30, 2025 at 10:46 AM Alexei Starovoitov
> >>> <alexei.starovoi...@gmail.com> wrote:
> >>>>
> >>>> On Sat, Apr 26, 2025 at 9:00 AM KaFai Wan <mannka...@gmail.com> wrote:
> >>>>>
> >>
>
> [...]
>
> >>
> >>
> >> bpf_get_func_arg() will be very helpful for bpfsnoop[1] when tracing
> >> tp_btf.
> >>
> >> In bpfsnoop, it can generate a small snippet of bpf instructions to use
> >> bpf_get_func_arg() for retrieving and filtering arguments. For example,
> >> with the netif_receive_skb tracepoint, bpfsnoop can use
> >> bpf_get_func_arg() to filter the skb argument using pcap-filter(7)[2] or
> >> a custom attribute-based filter. This will allow bpfsnoop to trace
> >> multiple tracepoints using a single bpf program code.
> >
> > I doubt you thought it through end to end.
> > When tracepoint prog attaches we have this check:
> > /*
> > * check that program doesn't access arguments beyond what's
> > * available in this tracepoint
> > */
> > if (prog->aux->max_ctx_offset > btp->num_args * sizeof(u64))
> > return -EINVAL;
> >
> > So you cannot have a single bpf prog attached to many tracepoints
> > to read many arguments as-is.
> > You can hack around that limit with probe_read,
> > but the values won't be trusted and you won't be able to pass
> > such untrusted pointers into skb and other helpers/kfuncs.
>
> I understand that a single bpf program cannot be attached to multiple
> tracepoints using tp_btf. However, the same bpf code can be reused to
> create multiple bpf programs, each attached to a different tracepoint.
>
> For example:
>
> SEC("fentry")
> int BPF_PROG(fentry_fn)
> {
> /* ... */
> return BPF_OK;
> }
>
> The above fentry code can be compiled into multiple bpf programs to
> trace different kernel functions. Each program can then use the
> bpf_get_func_arg() helper to access the arguments of the traced function.
>
> With this patch, tp_btf will gain similar flexibility. For example:
>
> SEC("tp_btf")
> int BPF_PROG(tp_btf_fn)
> {
> /* ... */
> return BPF_OK;
> }
>
> Here, bpf_get_func_arg() can be used to access tracepoint arguments.
>
> Currently, due to the lack of bpf_get_func_arg() support in tp_btf,
> bpfsnoop[1] uses bpf_probe_read_kernel() to read tracepoint arguments.
> This is also used when filtering specific argument attributes.
>
> For instance, to filter the skb argument of the netif_receive_skb
> tracepoint by 'skb->dev->ifindex == 2', the translated bpf instructions
> with bpf_probe_read_kernel() would look like this:
>
> bool filter_arg(__u64 * args):
> ; filter_arg(__u64 *args)
> 209: (79) r1 = *(u64 *)(r1 +0) /* all tracepoint's argument has been
> read into args using bpf_probe_read_kernel() */
> 210: (bf) r3 = r1
> 211: (07) r3 += 16
> 212: (b7) r2 = 8
> 213: (bf) r1 = r10
> 214: (07) r1 += -8
> 215: (85) call bpf_probe_read_kernel#-125280
> 216: (79) r3 = *(u64 *)(r10 -8)
> 217: (15) if r3 == 0x0 goto pc+10
> 218: (07) r3 += 224
> 219: (b7) r2 = 8
> 220: (bf) r1 = r10
> 221: (07) r1 += -8
> 222: (85) call bpf_probe_read_kernel#-125280
> 223: (79) r3 = *(u64 *)(r10 -8)
> 224: (67) r3 <<= 32
> 225: (77) r3 >>= 32
> 226: (b7) r0 = 1
> 227: (15) if r3 == 0x2 goto pc+1
> 228: (af) r0 ^= r0
> 229: (95) exit
>
> If bpf_get_func_arg() is supported in tp_btf, the bpf program will
> instead look like:
>
> static __noinline bool
> filter_skb(void *ctx)
> {
> struct sk_buff *skb;
>
> (void) bpf_get_func_arg(ctx, 0, (__u64 *) &skb);
> return skb->dev->ifindex == 2;
> }
>
> This will simplify the generated code and eliminate the need for
> bpf_probe_read_kernel() calls. However, in my tests (on kernel
> 6.8.0-35-generic, Ubuntu 24.04 LTS), the pointer returned by
> bpf_get_func_arg() is marked as a scalar rather than a trusted pointer:
>
> 0: R1=ctx() R10=fp0
> ; if (!filter_skb(ctx))
> 0: (85) call pc+3
> caller:
> R10=fp0
> callee:
> frame1: R1=ctx() R10=fp0
> 4: frame1: R1=ctx() R10=fp0
> ; filter_skb(void *ctx)
> 4: (bf) r3 = r10 ; frame1: R3_w=fp0 R10=fp0
> ;
> 5: (07) r3 += -8 ; frame1: R3_w=fp-8
> ; (void) bpf_get_func_arg(ctx, 0, (__u64 *) &skb);
> 6: (b7) r2 = 0 ; frame1: R2_w=0
> 7: (85) call bpf_get_func_arg#183 ; frame1: R0_w=scalar()
> ; return skb->dev->ifindex == 2;
> 8: (79) r1 = *(u64 *)(r10 -8) ; frame1: R1_w=scalar() R10=fp0
> fp-8=mmmmmmmm
> ; return skb->dev->ifindex == 2;
> 9: (79) r1 = *(u64 *)(r1 +16)
> R1 invalid mem access 'scalar'
> processed 7 insns (limit 1000000) max_states_per_insn 0 total_states 0
> peak_states 0 mark_read 0
>
> If the returned skb is a trusted pointer, the verifier will accept
> something like:
>
> static __noinline bool
> filter_skb(struct sk_buff *skb)
> {
> return skb->dev->ifindex == 2;
> }
>
> Which will compile into much simpler and more efficient instructions:
>
> bool filter_skb(struct sk_buff * skb):
> ; return skb->dev->ifindex == 2;
> 92: (79) r1 = *(u64 *)(r1 +16)
> ; return skb->dev->ifindex == 2;
> 93: (61) r1 = *(u32 *)(r1 +224)
> 94: (b7) r0 = 1
> ; return skb->dev->ifindex == 2;
> 95: (15) if r1 == 0x2 goto pc+1
> 96: (b7) r0 = 0
> ; return skb->dev->ifindex == 2;
> 97: (95) exit
>
> In conclusion:
>
> 1. It will be better if the pointer returned by bpf_get_func_arg() is
> trusted, only when the argument index is a known constant.
bpf_get_func_arg() was never meant to return trusted arguments, so
this, IMO, is pushing it too far.
> 2. Adding bpf_get_func_arg() support to tp_btf will significantly
> simplify and improve tools like bpfsnoop.
"Significantly simplify and improve" is a bit of an exaggeration,
given BPF cookies can be used for getting number of arguments of
tp_btf, as for the getting rid of bpf_probe_read_kernel(), tbh, more
generally useful addition would be an untyped counterpart to
bpf_core_cast(), which wouldn't need BTF type information, but will
treat all accessed memory as raw bytes (but will still install
exception handler just like with bpf_core_cast()).
>
> [1] https://github.com/bpfsnoop/bpfsnoop
>
> Thanks,
> Leon
>
>
