netdev - Re: [PATCH v1 bpf] net: Annotate rx_sk with __nullable for trace_kfree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <feb7ac0f-54e7-4e45-b79e-0fc8a4509437@linux.dev>
Date: Fri, 31 Jan 2025 18:19:22 -0800
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: Kumar Kartikeya Dwivedi <memxor@...il.com>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Andrii Nakryiko <andrii@...nel.org>, Kuniyuki Iwashima <kuni1840@...il.com>,
 bpf@...r.kernel.org, netdev@...r.kernel.org, Yan Zhai <yan@...udflare.com>
Subject: Re: [PATCH v1 bpf] net: Annotate rx_sk with __nullable for
 trace_kfree_skb.

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@gmail.com/

cc: Kumar