From: Martin KaFai Lau <martin.lau@xxxxxxxxx> Date: Fri, 31 Jan 2025 18:19:22 -0800 > On 1/31/25 4:14 PM, Kuniyuki Iwashima wrote: > > Yan Zhai reported a BPF prog could trigger a null-ptr-deref [0] > > in trace_kfree_skb if the prog does not check if rx_sk is NULL. > > > > Commit c53795d48ee8 ("net: add rx_sk to trace_kfree_skb") added > > rx_sk to trace_kfree_skb, but rx_sk is optional and could be NULL. > > > > Let's add __nullable suffix to rx_sk to let the BPF verifier > > validate such a prog and prevent the issue. > > > > Now we fail to load such a prog: > > > > libbpf: prog 'drop': -- BEGIN PROG LOAD LOG -- > > 0: R1=ctx() R10=fp0 > > ; int BPF_PROG(drop, struct sk_buff *skb, void *location, @ kfree_skb_sk_null.bpf.c:21 > > 0: (79) r3 = *(u64 *)(r1 +24) > > func 'kfree_skb' arg3 has btf_id 5253 type STRUCT 'sock' > > 1: R1=ctx() R3_w=trusted_ptr_or_null_sock(id=1) > > ; bpf_printk("sk: %d, %d\n", sk, sk->__sk_common.skc_family); @ kfree_skb_sk_null.bpf.c:24 > > 1: (69) r4 = *(u16 *)(r3 +16) > > R3 invalid mem access 'trusted_ptr_or_null_' > > processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > > -- END PROG LOAD LOG -- > > > > Note this fix requires commit 8aeaed21befc ("bpf: Support > > __nullable argument suffix for tp_btf"). > > I believe the current way is to add kfree_skb to the raw_tp_null_args[], > https://lore.kernel.org/all/20241213221929.3495062-3-memxor@xxxxxxxxx/ Oh, this is nice, thanks Martin! I was wondering if other explicit NULL-able args should be renamed, but looks like this series fixed all. Will post this as v2. ---8<--- diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 9de6acddd479..c3223e0db2f5 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6507,6 +6507,8 @@ static const struct bpf_raw_tp_null_args raw_tp_null_args[] = { /* rxrpc */ { "rxrpc_recvdata", 0x1 }, { "rxrpc_resend", 0x10 }, + /* skb */ + {"kfree_skb", 0x1000}, /* sunrpc */ { "xs_stream_read_data", 0x1 }, /* ... from xprt_cong_event event class */ ---8<--- Thanks!