[NETFILTER 2.6.25 1/1]: ipt_CLUSTERIP: fix race between clusterip_config_find_get and _entry_put

[NETFILTER 2.6.25 1/1]: ipt_CLUSTERIP: fix race between clusterip_config_find_get and _entry_put

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2169 bytes --]

commit 441771f9b568a1d8dafcd556aca21b1386e2858c
Author: Pavel Emelyanov <xemul@openvz.org>
Date:   Sun Apr 13 06:54:33 2008 +0200

    [NETFILTER]: ipt_CLUSTERIP: fix race between clusterip_config_find_get and _entry_put
    
    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>

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 52926c8..a12dd32 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -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 *

Re: [NETFILTER 2.6.25 1/1]: ipt_CLUSTERIP: fix race between clusterip_config_find_get and _entry_put

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Sun, 13 Apr 2008 22:21:59 +0200

> [NETFILTER]: ipt_CLUSTERIP: fix race between clusterip_config_find_get and _entry_put

Applied, thanks.