[NETFILTER]: conntrack: fix race condition in early_drop
authorPablo Neira Ayuso <pablo@netfilter.org>
Wed, 20 Sep 2006 19:01:06 +0000 (12:01 -0700)
committerDavid S. Miller <davem@sunset.davemloft.net>
Fri, 22 Sep 2006 22:19:54 +0000 (15:19 -0700)
On SMP environments the maximum number of conntracks can be overpassed
under heavy stress situations due to an existing race condition.

        CPU A                   CPU B
     atomic_read()               ...
     early_drop()                ...
        ...                  atomic_read()
   allocate conntrack      allocate conntrack
     atomic_inc()             atomic_inc()

This patch moves the counter incrementation before the early drop stage.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/ipv4/netfilter/ip_conntrack_core.c
net/netfilter/nf_conntrack_core.c

index 2568d480e9a92a6b6e78a0727fcac449693fb44e..422a662194cc425f22a718766a63aace2d7cce59 100644 (file)
@@ -622,11 +622,15 @@ struct ip_conntrack *ip_conntrack_alloc(struct ip_conntrack_tuple *orig,
                ip_conntrack_hash_rnd_initted = 1;
        }
 
+       /* We don't want any race condition at early drop stage */
+       atomic_inc(&ip_conntrack_count);
+
        if (ip_conntrack_max
-           && atomic_read(&ip_conntrack_count) >= ip_conntrack_max) {
+           && atomic_read(&ip_conntrack_count) > ip_conntrack_max) {
                unsigned int hash = hash_conntrack(orig);
                /* Try dropping from this hash chain. */
                if (!early_drop(&ip_conntrack_hash[hash])) {
+                       atomic_dec(&ip_conntrack_count);
                        if (net_ratelimit())
                                printk(KERN_WARNING
                                       "ip_conntrack: table full, dropping"
@@ -638,6 +642,7 @@ struct ip_conntrack *ip_conntrack_alloc(struct ip_conntrack_tuple *orig,
        conntrack = kmem_cache_alloc(ip_conntrack_cachep, GFP_ATOMIC);
        if (!conntrack) {
                DEBUGP("Can't allocate conntrack.\n");
+               atomic_dec(&ip_conntrack_count);
                return ERR_PTR(-ENOMEM);
        }
 
@@ -651,8 +656,6 @@ struct ip_conntrack *ip_conntrack_alloc(struct ip_conntrack_tuple *orig,
        conntrack->timeout.data = (unsigned long)conntrack;
        conntrack->timeout.function = death_by_timeout;
 
-       atomic_inc(&ip_conntrack_count);
-
        return conntrack;
 }
 
index 927137b8b3b5d520339743f9b197c5e310682d02..adeafa2cc339ea8773750d08f0250df7e64686be 100644 (file)
@@ -848,11 +848,15 @@ __nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
                nf_conntrack_hash_rnd_initted = 1;
        }
 
+       /* We don't want any race condition at early drop stage */
+       atomic_inc(&nf_conntrack_count);
+
        if (nf_conntrack_max
-           && atomic_read(&nf_conntrack_count) >= nf_conntrack_max) {
+           && atomic_read(&nf_conntrack_count) > nf_conntrack_max) {
                unsigned int hash = hash_conntrack(orig);
                /* Try dropping from this hash chain. */
                if (!early_drop(&nf_conntrack_hash[hash])) {
+                       atomic_dec(&nf_conntrack_count);
                        if (net_ratelimit())
                                printk(KERN_WARNING
                                       "nf_conntrack: table full, dropping"
@@ -903,10 +907,12 @@ __nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
        init_timer(&conntrack->timeout);
        conntrack->timeout.data = (unsigned long)conntrack;
        conntrack->timeout.function = death_by_timeout;
+       read_unlock_bh(&nf_ct_cache_lock);
 
-       atomic_inc(&nf_conntrack_count);
+       return conntrack;
 out:
        read_unlock_bh(&nf_ct_cache_lock);
+       atomic_dec(&nf_conntrack_count);
        return conntrack;
 }