• Liping Zhang's avatar
    netfilter: ctnetlink: make it safer when updating ct->status · 53b56da8
    Liping Zhang authored
    After converting to use rcu for conntrack hash, one CPU may update
    the ct->status via ctnetlink, while another CPU may process the
    packets and update the ct->status.
    
    So the non-atomic operation "ct->status |= status;" via ctnetlink
    becomes unsafe, and this may clear the IPS_DYING_BIT bit set by
    another CPU unexpectedly. For example:
             CPU0                            CPU1
      ctnetlink_change_status        __nf_conntrack_find_get
          old = ct->status              nf_ct_gc_expired
              -                         nf_ct_kill
              -                      test_and_set_bit(IPS_DYING_BIT
          new = old | status;                 -
      ct->status = new; <-- oops, _DYING_ is cleared!
    
    Now using a series of atomic bit operation to solve the above issue.
    
    Also note, user shouldn't set IPS_TEMPLATE, IPS_SEQ_ADJUST directly,
    so make these two bits be unchangable too.
    
    If we set the IPS_TEMPLATE_BIT, ct will be freed by nf_ct_tmpl_free,
    but actually it is alloced by nf_conntrack_alloc.
    If we set the IPS_SEQ_ADJUST_BIT, this may cause the NULL pointer
    deference, as the nfct_seqadj(ct) maybe NULL.
    
    Last, add some comments to describe the logic change due to the
    commit a963d710 ("netfilter: ctnetlink: Fix regression in CTA_STATUS
    processing"), which makes me feel a little confusing.
    
    Fixes: 76507f69 ("[NETFILTER]: nf_conntrack: use RCU for conntrack hash")
    Signed-off-by: default avatarLiping Zhang <zlpnobody@gmail.com>
    Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
    53b56da8
nf_conntrack_netlink.c 83.6 KB