[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2024090543-itinerary-marina-3814@gregkh>
Date: Thu, 5 Sep 2024 09:43:55 +0200
From: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To: Siddh Raman Pant <siddh.raman.pant@...cle.com>
Cc: "stable@...r.kernel.org" <stable@...r.kernel.org>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 5.10, 5.4] net: set SOCK_RCU_FREE before inserting socket
into hashtable
On Wed, Sep 04, 2024 at 01:06:45PM +0000, Siddh Raman Pant wrote:
> [ Upstream commit 871019b22d1bcc9fab2d1feba1b9a564acbb6e99 ]
>
> We've started to see the following kernel traces:
>
> WARNING: CPU: 83 PID: 0 at net/core/filter.c:6641 sk_lookup+0x1bd/0x1d0
>
> Call Trace:
> <IRQ>
> __bpf_skc_lookup+0x10d/0x120
> bpf_sk_lookup+0x48/0xd0
> bpf_sk_lookup_tcp+0x19/0x20
> bpf_prog_<redacted>+0x37c/0x16a3
> cls_bpf_classify+0x205/0x2e0
> tcf_classify+0x92/0x160
> __netif_receive_skb_core+0xe52/0xf10
> __netif_receive_skb_list_core+0x96/0x2b0
> napi_complete_done+0x7b5/0xb70
> <redacted>_poll+0x94/0xb0
> net_rx_action+0x163/0x1d70
> __do_softirq+0xdc/0x32e
> asm_call_irq_on_stack+0x12/0x20
> </IRQ>
> do_softirq_own_stack+0x36/0x50
> do_softirq+0x44/0x70
>
> __inet_hash can race with lockless (rcu) readers on the other cpus:
>
> __inet_hash
> __sk_nulls_add_node_rcu
> <- (bpf triggers here)
> sock_set_flag(SOCK_RCU_FREE)
>
> Let's move the SOCK_RCU_FREE part up a bit, before we are inserting
> the socket into hashtables. Note, that the race is really harmless;
> the bpf callers are handling this situation (where listener socket
> doesn't have SOCK_RCU_FREE set) correctly, so the only
> annoyance is a WARN_ONCE.
>
> More details from Eric regarding SOCK_RCU_FREE timeline:
>
> Commit 3b24d854cb35 ("tcp/dccp: do not touch listener sk_refcnt under
> synflood") added SOCK_RCU_FREE. At that time, the precise location of
> sock_set_flag(sk, SOCK_RCU_FREE) did not matter, because the thread calling
> __inet_hash() owns a reference on sk. SOCK_RCU_FREE was only tested
> at dismantle time.
>
> Commit 6acc9b432e67 ("bpf: Add helper to retrieve socket in BPF")
> started checking SOCK_RCU_FREE _after_ the lookup to infer whether
> the refcount has been taken care of.
>
> Fixes: 6acc9b432e67 ("bpf: Add helper to retrieve socket in BPF")
> Reviewed-by: Eric Dumazet <edumazet@...gle.com>
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> Signed-off-by: David S. Miller <davem@...emloft.net>
> [Resolved conflict for 5.10 and below.]
> Signed-off-by: Siddh Raman Pant <siddh.raman.pant@...cle.com>
> ---
> net/ipv4/inet_hashtables.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Now queued up, thanks.
greg k-h
Powered by blists - more mailing lists