Re: [Patch] ipvs: close race conditions on ip_vs_conn_tab list modification

2 messages, 2 authors, 2005-06-26 · open the first message on its own page

Re: [Patch] ipvs: close race conditions on ip_vs_conn_tab list modification

From: Neil Horman <hidden>
Date: 2005-06-24 17:43:02

On Fri, Jun 24, 2005 at 06:09:40PM +0300, Julian Anastasov wrote:
	Hello,

On Fri, 24 Jun 2005, Neil Horman wrote:
quoted
 			if (ct) {
 				IP_VS_DBG(4, "del conn template\n");
 				ip_vs_conn_expire_now(ct);
 			}
	Don't forget to use cp->control instead of ct, ct is not needed
anymore.

Regards

--
Julian Anastasov [off-list ref]

Good catch.  Sorry, should have seen that earlier.  New patch attached with
corrections.  When you're comfortable with this, I'll post the 2.4 version of
the patch.

Regards 
Neil

Signed-off-by: Neil Horman <redacted>

 ip_vs_conn.c |   24 ++++--------------------
 1 files changed, 4 insertions(+), 20 deletions(-)

--- linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c.orig	2005-06-23 13:11:00.000000000 -0400
+++ linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c	2005-06-24 13:33:03.000000000 -0400
@@ -548,7 +548,6 @@
 {
 	if (del_timer(&cp->timer))
 		mod_timer(&cp->timer, jiffies);
-	__ip_vs_conn_put(cp);
 }
 
 
@@ -801,21 +800,12 @@
 					continue;
 			}
 
-			/*
-			 * Drop the entry, and drop its ct if not referenced
-			 */
-			atomic_inc(&cp->refcnt);
-			ct_write_unlock(hash);
-
-			if ((ct = cp->control))
-				atomic_inc(&ct->refcnt);
 			IP_VS_DBG(4, "del connection\n");
 			ip_vs_conn_expire_now(cp);
-			if (ct) {
+			if (cp->control) {
 				IP_VS_DBG(4, "del conn template\n");
-				ip_vs_conn_expire_now(ct);
+				ip_vs_conn_expire_now(cp->control);
 			}
-			ct_write_lock(hash);
 		}
 		ct_write_unlock(hash);
 	}
@@ -829,7 +819,6 @@
 {
 	int idx;
 	struct ip_vs_conn *cp;
-	struct ip_vs_conn *ct;
 
   flush_again:
 	for (idx=0; idx<IP_VS_CONN_TAB_SIZE; idx++) {
@@ -839,18 +828,13 @@
 		ct_write_lock_bh(idx);
 
 		list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
-			atomic_inc(&cp->refcnt);
-			ct_write_unlock(idx);
 
-			if ((ct = cp->control))
-				atomic_inc(&ct->refcnt);
 			IP_VS_DBG(4, "del connection\n");
 			ip_vs_conn_expire_now(cp);
-			if (ct) {
+			if (cp->control) {
 				IP_VS_DBG(4, "del conn template\n");
-				ip_vs_conn_expire_now(ct);
+				ip_vs_conn_expire_now(cp->control);
 			}
-			ct_write_lock(idx);
 		}
 		ct_write_unlock_bh(idx);
 	}
-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

Re: [Patch] ipvs: close race conditions on ip_vs_conn_tab list modification

From: Wensong Zhang <hidden>
Date: 2005-06-26 17:05:55

Hello Neil,

It's a good fix. Please continue on kernel 2.4 version of your patch.

Thanks,

Wensong


On Fri, 24 Jun 2005, Neil Horman wrote:
quoted hunk
On Fri, Jun 24, 2005 at 06:09:40PM +0300, Julian Anastasov wrote:
quoted
	Hello,

On Fri, 24 Jun 2005, Neil Horman wrote:
quoted
 			if (ct) {
 				IP_VS_DBG(4, "del conn template\n");
 				ip_vs_conn_expire_now(ct);
 			}
	Don't forget to use cp->control instead of ct, ct is not needed
anymore.

Regards

--
Julian Anastasov [off-list ref]

Good catch.  Sorry, should have seen that earlier.  New patch attached with
corrections.  When you're comfortable with this, I'll post the 2.4 version of
the patch.

Regards
Neil

Signed-off-by: Neil Horman <redacted>

ip_vs_conn.c |   24 ++++--------------------
1 files changed, 4 insertions(+), 20 deletions(-)

--- linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c.orig	2005-06-23 13:11:00.000000000 -0400
+++ linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c	2005-06-24 13:33:03.000000000 -0400
@@ -548,7 +548,6 @@
{
	if (del_timer(&cp->timer))
		mod_timer(&cp->timer, jiffies);
-	__ip_vs_conn_put(cp);
}

@@ -801,21 +800,12 @@
					continue;
			}

-			/*
-			 * Drop the entry, and drop its ct if not referenced
-			 */
-			atomic_inc(&cp->refcnt);
-			ct_write_unlock(hash);
-
-			if ((ct = cp->control))
-				atomic_inc(&ct->refcnt);
			IP_VS_DBG(4, "del connection\n");
			ip_vs_conn_expire_now(cp);
-			if (ct) {
+			if (cp->control) {
				IP_VS_DBG(4, "del conn template\n");
-				ip_vs_conn_expire_now(ct);
+				ip_vs_conn_expire_now(cp->control);
			}
-			ct_write_lock(hash);
		}
		ct_write_unlock(hash);
	}
@@ -829,7 +819,6 @@
{
	int idx;
	struct ip_vs_conn *cp;
-	struct ip_vs_conn *ct;

  flush_again:
	for (idx=0; idx<IP_VS_CONN_TAB_SIZE; idx++) {
@@ -839,18 +828,13 @@
		ct_write_lock_bh(idx);

		list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
-			atomic_inc(&cp->refcnt);
-			ct_write_unlock(idx);

-			if ((ct = cp->control))
-				atomic_inc(&ct->refcnt);
			IP_VS_DBG(4, "del connection\n");
			ip_vs_conn_expire_now(cp);
-			if (ct) {
+			if (cp->control) {
				IP_VS_DBG(4, "del conn template\n");
-				ip_vs_conn_expire_now(ct);
+				ip_vs_conn_expire_now(cp->control);
			}
-			ct_write_lock(idx);
		}
		ct_write_unlock_bh(idx);
	}
-- 
/***************************************************
*Neil Horman
*Software Engineer
*Red Hat, Inc.
*nhorman@redhat.com
*gpg keyid: 1024D / 0x92A74FA1
*http://pgp.mit.edu
***************************************************/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help