Re: [PATCH v1 bpf] net: Annotate rx_sk with __nullable for trace_kfree_skb.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 31, 2025 at 8:19 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> 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/
>
Nice to learn the trick. Thanks Martin!

Yan

> cc: Kumar
>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux