• Stanislav Fomichev's avatar
    net: set SOCK_RCU_FREE before inserting socket into hashtable · 871019b2
    Stanislav Fomichev authored
    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 3b24d854 ("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 6acc9b43 ("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: 6acc9b43 ("bpf: Add helper to retrieve socket in BPF")
    Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
    Signed-off-by: default avatarStanislav Fomichev <sdf@google.com>
    Reviewed-by: default avatarKuniyuki Iwashima <kuniyu@amazon.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    871019b2
inet_hashtables.c 33.8 KB