netfilter: nf_nat: fix RCU races
Patrick McHardy [Tue, 17 Jun 2008 22:51:47 +0000 (15:51 -0700)]
Fix three ct_extend/NAT extension related races:

- When cleaning up the extension area and removing it from the bysource hash,
  the nat->ct pointer must not be set to NULL since it may still be used in
  a RCU read side

- When replacing a NAT extension area in the bysource hash, the nat->ct
  pointer must be assigned before performing the replacement

- When reallocating extension storage in ct_extend, the old memory must
  not be freed immediately since it may still be used by a RCU read side

Possibly fixes https://bugzilla.redhat.com/show_bug.cgi?id=449315
and/or http://bugzilla.kernel.org/show_bug.cgi?id=10875

Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

include/net/netfilter/nf_conntrack_extend.h
net/ipv4/netfilter/nf_nat_core.c
net/netfilter/nf_conntrack_extend.c

index f736e84..f80c0ed 100644 (file)
@@ -15,6 +15,7 @@ enum nf_ct_ext_id
 
 /* Extensions: optional stuff which isn't permanently in struct. */
 struct nf_ct_ext {
+       struct rcu_head rcu;
        u8 offset[NF_CT_EXT_NUM];
        u8 len;
        char data[0];
index 0457859..d2a887f 100644 (file)
@@ -556,7 +556,6 @@ static void nf_nat_cleanup_conntrack(struct nf_conn *ct)
 
        spin_lock_bh(&nf_nat_lock);
        hlist_del_rcu(&nat->bysource);
-       nat->ct = NULL;
        spin_unlock_bh(&nf_nat_lock);
 }
 
@@ -570,8 +569,8 @@ static void nf_nat_move_storage(void *new, void *old)
                return;
 
        spin_lock_bh(&nf_nat_lock);
-       hlist_replace_rcu(&old_nat->bysource, &new_nat->bysource);
        new_nat->ct = ct;
+       hlist_replace_rcu(&old_nat->bysource, &new_nat->bysource);
        spin_unlock_bh(&nf_nat_lock);
 }
 
index bcc19fa..8a3f8b3 100644 (file)
@@ -59,12 +59,19 @@ nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id, gfp_t gfp)
        if (!*ext)
                return NULL;
 
+       INIT_RCU_HEAD(&(*ext)->rcu);
        (*ext)->offset[id] = off;
        (*ext)->len = len;
 
        return (void *)(*ext) + off;
 }
 
+static void __nf_ct_ext_free_rcu(struct rcu_head *head)
+{
+       struct nf_ct_ext *ext = container_of(head, struct nf_ct_ext, rcu);
+       kfree(ext);
+}
+
 void *__nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp)
 {
        struct nf_ct_ext *new;
@@ -106,7 +113,7 @@ void *__nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp)
                                        (void *)ct->ext + ct->ext->offset[i]);
                        rcu_read_unlock();
                }
-               kfree(ct->ext);
+               call_rcu(&ct->ext->rcu, __nf_ct_ext_free_rcu);
                ct->ext = new;
        }