[NETFILTER]: ipt_CLUSTERIP: fix race between clusterip_config_find_get and _entry_put
authorPavel Emelyanov <xemul@openvz.org>
Mon, 14 Apr 2008 07:44:52 +0000 (00:44 -0700)
committerDavid S. Miller <davem@davemloft.net>
Mon, 14 Apr 2008 07:44:52 +0000 (00:44 -0700)
Consider we are putting a clusterip_config entry with the "entries"
count == 1, and on the other CPU there's a clusterip_config_find_get
in progress:

CPU1: CPU2:
clusterip_config_entry_put: clusterip_config_find_get:
if (atomic_dec_and_test(&c->entries)) {
/* true */
read_lock_bh(&clusterip_lock);
c = __clusterip_config_find(clusterip);
/* found - it's still in list */
...
atomic_inc(&c->entries);
read_unlock_bh(&clusterip_lock);

write_lock_bh(&clusterip_lock);
list_del(&c->list);
write_unlock_bh(&clusterip_lock);
...
dev_put(c->dev);

Oops! We have an entry returned by the clusterip_config_find_get,
which is a) not in list b) has a stale dev pointer.

The problems will happen when the CPU2 will release the entry - it
will remove it from the list for the 2nd time, thus spoiling it, and
will put a stale dev pointer.

The fix is to make atomic_dec_and_test under the clusterip_lock.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: Patrick McHardy <kaber@trash.net>
net/ipv4/netfilter/ipt_CLUSTERIP.c

index 52926c8..a12dd32 100644 (file)
@@ -82,8 +82,8 @@ clusterip_config_put(struct clusterip_config *c)
 static inline void
 clusterip_config_entry_put(struct clusterip_config *c)
 {
+       write_lock_bh(&clusterip_lock);
        if (atomic_dec_and_test(&c->entries)) {
-               write_lock_bh(&clusterip_lock);
                list_del(&c->list);
                write_unlock_bh(&clusterip_lock);
 
@@ -96,7 +96,9 @@ clusterip_config_entry_put(struct clusterip_config *c)
 #ifdef CONFIG_PROC_FS
                remove_proc_entry(c->pde->name, c->pde->parent);
 #endif
+               return;
        }
+       write_unlock_bh(&clusterip_lock);
 }
 
 static struct clusterip_config *