Sophie

Sophie

distrib > Scientific%20Linux > 5x > x86_64 > by-pkgid > 27922b4260f65d317aabda37e42bbbff > files > 2329

kernel-2.6.18-238.el5.src.rpm

From 049b35560b2cc7facc246f78134bbd68caa37e34 Mon Sep 17 00:00:00 2001
From: Fabio Olive Leite <fleite@redhat.com>
Date: Thu, 3 Jul 2008 17:11:39 -0400
Subject: [PATCH] [net] Fixing bonding rtnl_lock screwups

Message-id: <20080605221454.GB5901@sleipnir.ol>
O-Subject: [5.3 NET BONDING BZ450219] Fixing bonding rtnl_lock screwups
Bugzilla: 450219


Hi all,

Between 5.1 and 5.2 (especially after -62.el5) some changes to the
bonding driver left unbalanced rtnl_lock and rtnl_unlock calls in the
sysfs interface code.

Specifically, it is possible for bonding_store_bonds() to return
without unlocking rtnl and for bonding_store_slaves() to do an unlock
without going through the codepath that locks.

The patch below is being tested at a customer right now. They were
seeing some bonding errors followed by several RTNL assertion errors.

The fix is partly based on upstream commit
c4ebc66a1a8e3576322a9f47f0d06ec3c96a08d7.

I believe we should do a 5.2.Z as well, as it was broken on 5.2.

Please let me know if the fix below is suboptimal.

Acked-by: Neil Horman <nhorman@redhat.com>
Acked-by: Rik van Riel <riel@redhat.com>
Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Andy Gospodarek <gospo@redhat.com>
Acked-by: "David S. Miller" <davem@redhat.com>
---
 drivers/net/bonding/bond_sysfs.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index dbd9b96..b1f861f 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -147,29 +147,29 @@ static ssize_t bonding_store_bonds(struct class *cls, const char *buffer, size_t
 						": Unable remove bond %s due to open references.\n",
 						ifname);
 					res = -EPERM;
-					goto out;
+					goto out_unlock;
 				}
 				printk(KERN_INFO DRV_NAME
 					": %s is being deleted...\n",
 					bond->dev->name);
 				bond_destroy(bond);
-				up_write(&bonding_rwsem);
-				rtnl_unlock();
-				goto out;
+				goto out_unlock;
 			}
 
 		printk(KERN_ERR DRV_NAME
 			": unable to delete non-existent bond %s\n", ifname);
 		res = -ENODEV;
-		up_write(&bonding_rwsem);
-		rtnl_unlock();
-		goto out;
+		goto out_unlock;
 	}
 
 err_no_cmd:
 	printk(KERN_ERR DRV_NAME
 		": no command found in bonding_masters. Use +ifname or -ifname.\n");
-	res = -EPERM;
+	return -EPERM;
+
+out_unlock:
+	up_write(&bonding_rwsem);
+	rtnl_unlock();
 
 	/* Always return either count or an error.  If you return 0, you'll
 	 * get called forever, which is bad.
@@ -254,8 +254,8 @@ static ssize_t bonding_store_slaves(struct class_device *cd, const char *buffer,
 		printk(KERN_ERR DRV_NAME
 		       ": %s: Unable to update slaves because interface is down.\n",
 		       bond->dev->name);
-		ret = -EPERM;
-		goto out;
+		/* early return before rtnl_lock() */
+		return -EPERM;
 	}
 
 	/* Note:  We can't hold bond->lock here, as bond_create grabs it. */
-- 
1.5.5.1