Skip to content
  • Liping Zhang's avatar
    netfilter: nf_ct_ext: fix possible panic after nf_ct_extend_unregister · 5170d210
    Liping Zhang authored
    commit 9c3f3794
    
     upstream.
    
    If one cpu is doing nf_ct_extend_unregister while another cpu is doing
    __nf_ct_ext_add_length, then we may hit BUG_ON(t == NULL). Moreover,
    there's no synchronize_rcu invocation after set nf_ct_ext_types[id] to
    NULL, so it's possible that we may access invalid pointer.
    
    But actually, most of the ct extends are built-in, so the problem listed
    above will not happen. However, there are two exceptions: NF_CT_EXT_NAT
    and NF_CT_EXT_SYNPROXY.
    
    For _EXT_NAT, the panic will not happen, since adding the nat extend and
    unregistering the nat extend are located in the same file(nf_nat_core.c),
    this means that after the nat module is removed, we cannot add the nat
    extend too.
    
    For _EXT_SYNPROXY, synproxy extend may be added by init_conntrack, while
    synproxy extend unregister will be done by synproxy_core_exit. So after
    nf_synproxy_core.ko is removed, we may still try to add the synproxy
    extend, then kernel panic may happen.
    
    I know it's very hard to reproduce this issue, but I can play a tricky
    game to make it happen very easily :)
    
    Step 1. Enable SYNPROXY for tcp dport 1234 at FORWARD hook:
      # iptables -I FORWARD -p tcp --dport 1234 -j SYNPROXY
    Step 2. Queue the syn packet to the userspace at raw table OUTPUT hook.
            Also note, in the userspace we only add a 20s' delay, then
            reinject the syn packet to the kernel:
      # iptables -t raw -I OUTPUT -p tcp --syn -j NFQUEUE --queue-num 1
    Step 3. Using "nc 2.2.2.2 1234" to connect the server.
    Step 4. Now remove the nf_synproxy_core.ko quickly:
      # iptables -F FORWARD
      # rmmod ipt_SYNPROXY
      # rmmod nf_synproxy_core
    Step 5. After 20s' delay, the syn packet is reinjected to the kernel.
    
    Now you will see the panic like this:
      kernel BUG at net/netfilter/nf_conntrack_extend.c:91!
      Call Trace:
       ? __nf_ct_ext_add_length+0x53/0x3c0 [nf_conntrack]
       init_conntrack+0x12b/0x600 [nf_conntrack]
       nf_conntrack_in+0x4cc/0x580 [nf_conntrack]
       ipv4_conntrack_local+0x48/0x50 [nf_conntrack_ipv4]
       nf_reinject+0x104/0x270
       nfqnl_recv_verdict+0x3e1/0x5f9 [nfnetlink_queue]
       ? nfqnl_recv_verdict+0x5/0x5f9 [nfnetlink_queue]
       ? nla_parse+0xa0/0x100
       nfnetlink_rcv_msg+0x175/0x6a9 [nfnetlink]
       [...]
    
    One possible solution is to make NF_CT_EXT_SYNPROXY extend built-in, i.e.
    introduce nf_conntrack_synproxy.c and only do ct extend register and
    unregister in it, similar to nf_conntrack_timeout.c.
    
    But having such a obscure restriction of nf_ct_extend_unregister is not a
    good idea, so we should invoke synchronize_rcu after set nf_ct_ext_types
    to NULL, and check the NULL pointer when do __nf_ct_ext_add_length. Then
    it will be easier if we add new ct extend in the future.
    
    Last, we use kfree_rcu to free nf_ct_ext, so rcu_barrier() is unnecessary
    anymore, remove it too.
    
    Signed-off-by: default avatarLiping Zhang <zlpnobody@gmail.com>
    Acked-by: default avatarFlorian Westphal <fw@strlen.de>
    Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
    Cc: Stefan Bader <stefan.bader@canonical.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    5170d210