aboutsummaryrefslogtreecommitdiffstats
path: root/drivers
diff options
context:
space:
mode:
authorJay Vosburgh <fubar@us.ibm.com>2008-01-17 19:25:02 -0500
committerJeff Garzik <jeff@garzik.org>2008-01-18 14:38:39 -0500
commit027ea0416c955778ceca7ef82e48a1dd6b4617c9 (patch)
treead823fb28b414d948fc372a08c8c15cee31bd8e1 /drivers
parentece95f7fefe3afae19e641e1b3f5e64b00d5b948 (diff)
bonding: fix lock ordering for rtnl and bonding_rwsem
Fix the handling of rtnl and the bonding_rwsem to always be acquired in a consistent order (rtnl, then bonding_rwsem). The existing code sometimes acquired them in this order, and sometimes in the opposite order, which opens a window for deadlock between ifenslave and sysfs. Signed-off-by: Jay Vosburgh <fubar@us.ibm.com> Signed-off-by: Jeff Garzik <jeff@garzik.org>
Diffstat (limited to 'drivers')
-rw-r--r--drivers/net/bonding/bond_main.c19
-rw-r--r--drivers/net/bonding/bond_sysfs.c43
2 files changed, 35 insertions, 27 deletions
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 379c5d87c804..2c6da4969382 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4874,9 +4874,22 @@ static struct lock_class_key bonding_netdev_xmit_lock_key;
4874int bond_create(char *name, struct bond_params *params, struct bonding **newbond) 4874int bond_create(char *name, struct bond_params *params, struct bonding **newbond)
4875{ 4875{
4876 struct net_device *bond_dev; 4876 struct net_device *bond_dev;
4877 struct bonding *bond, *nxt;
4877 int res; 4878 int res;
4878 4879
4879 rtnl_lock(); 4880 rtnl_lock();
4881 down_write(&bonding_rwsem);
4882
4883 /* Check to see if the bond already exists. */
4884 list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list)
4885 if (strnicmp(bond->dev->name, name, IFNAMSIZ) == 0) {
4886 printk(KERN_ERR DRV_NAME
4887 ": cannot add bond %s; it already exists\n",
4888 name);
4889 res = -EPERM;
4890 goto out_rtnl;
4891 }
4892
4880 bond_dev = alloc_netdev(sizeof(struct bonding), name ? name : "", 4893 bond_dev = alloc_netdev(sizeof(struct bonding), name ? name : "",
4881 ether_setup); 4894 ether_setup);
4882 if (!bond_dev) { 4895 if (!bond_dev) {
@@ -4915,10 +4928,12 @@ int bond_create(char *name, struct bond_params *params, struct bonding **newbond
4915 4928
4916 netif_carrier_off(bond_dev); 4929 netif_carrier_off(bond_dev);
4917 4930
4931 up_write(&bonding_rwsem);
4918 rtnl_unlock(); /* allows sysfs registration of net device */ 4932 rtnl_unlock(); /* allows sysfs registration of net device */
4919 res = bond_create_sysfs_entry(bond_dev->priv); 4933 res = bond_create_sysfs_entry(bond_dev->priv);
4920 if (res < 0) { 4934 if (res < 0) {
4921 rtnl_lock(); 4935 rtnl_lock();
4936 down_write(&bonding_rwsem);
4922 goto out_bond; 4937 goto out_bond;
4923 } 4938 }
4924 4939
@@ -4929,6 +4944,7 @@ out_bond:
4929out_netdev: 4944out_netdev:
4930 free_netdev(bond_dev); 4945 free_netdev(bond_dev);
4931out_rtnl: 4946out_rtnl:
4947 up_write(&bonding_rwsem);
4932 rtnl_unlock(); 4948 rtnl_unlock();
4933 return res; 4949 return res;
4934} 4950}
@@ -4949,6 +4965,9 @@ static int __init bonding_init(void)
4949#ifdef CONFIG_PROC_FS 4965#ifdef CONFIG_PROC_FS
4950 bond_create_proc_dir(); 4966 bond_create_proc_dir();
4951#endif 4967#endif
4968
4969 init_rwsem(&bonding_rwsem);
4970
4952 for (i = 0; i < max_bonds; i++) { 4971 for (i = 0; i < max_bonds; i++) {
4953 res = bond_create(NULL, &bonding_defaults, NULL); 4972 res = bond_create(NULL, &bonding_defaults, NULL);
4954 if (res) 4973 if (res)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index bff4f2b84ce0..90a1f31e8e63 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -109,11 +109,10 @@ static ssize_t bonding_store_bonds(struct class *cls, const char *buffer, size_t
109{ 109{
110 char command[IFNAMSIZ + 1] = {0, }; 110 char command[IFNAMSIZ + 1] = {0, };
111 char *ifname; 111 char *ifname;
112 int res = count; 112 int rv, res = count;
113 struct bonding *bond; 113 struct bonding *bond;
114 struct bonding *nxt; 114 struct bonding *nxt;
115 115
116 down_write(&(bonding_rwsem));
117 sscanf(buffer, "%16s", command); /* IFNAMSIZ*/ 116 sscanf(buffer, "%16s", command); /* IFNAMSIZ*/
118 ifname = command + 1; 117 ifname = command + 1;
119 if ((strlen(command) <= 1) || 118 if ((strlen(command) <= 1) ||
@@ -121,39 +120,28 @@ static ssize_t bonding_store_bonds(struct class *cls, const char *buffer, size_t
121 goto err_no_cmd; 120 goto err_no_cmd;
122 121
123 if (command[0] == '+') { 122 if (command[0] == '+') {
124
125 /* Check to see if the bond already exists. */
126 list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list)
127 if (strnicmp(bond->dev->name, ifname, IFNAMSIZ) == 0) {
128 printk(KERN_ERR DRV_NAME
129 ": cannot add bond %s; it already exists\n",
130 ifname);
131 res = -EPERM;
132 goto out;
133 }
134
135 printk(KERN_INFO DRV_NAME 123 printk(KERN_INFO DRV_NAME
136 ": %s is being created...\n", ifname); 124 ": %s is being created...\n", ifname);
137 if (bond_create(ifname, &bonding_defaults, &bond)) { 125 rv = bond_create(ifname, &bonding_defaults, &bond);
138 printk(KERN_INFO DRV_NAME 126 if (rv) {
139 ": %s interface already exists. Bond creation failed.\n", 127 printk(KERN_INFO DRV_NAME ": Bond creation failed.\n");
140 ifname); 128 res = rv;
141 res = -EPERM;
142 } 129 }
143 goto out; 130 goto out;
144 } 131 }
145 132
146 if (command[0] == '-') { 133 if (command[0] == '-') {
134 rtnl_lock();
135 down_write(&bonding_rwsem);
136
147 list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list) 137 list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list)
148 if (strnicmp(bond->dev->name, ifname, IFNAMSIZ) == 0) { 138 if (strnicmp(bond->dev->name, ifname, IFNAMSIZ) == 0) {
149 rtnl_lock();
150 /* check the ref count on the bond's kobject. 139 /* check the ref count on the bond's kobject.
151 * If it's > expected, then there's a file open, 140 * If it's > expected, then there's a file open,
152 * and we have to fail. 141 * and we have to fail.
153 */ 142 */
154 if (atomic_read(&bond->dev->dev.kobj.kref.refcount) 143 if (atomic_read(&bond->dev->dev.kobj.kref.refcount)
155 > expected_refcount){ 144 > expected_refcount){
156 rtnl_unlock();
157 printk(KERN_INFO DRV_NAME 145 printk(KERN_INFO DRV_NAME
158 ": Unable remove bond %s due to open references.\n", 146 ": Unable remove bond %s due to open references.\n",
159 ifname); 147 ifname);
@@ -164,6 +152,7 @@ static ssize_t bonding_store_bonds(struct class *cls, const char *buffer, size_t
164 ": %s is being deleted...\n", 152 ": %s is being deleted...\n",
165 bond->dev->name); 153 bond->dev->name);
166 bond_destroy(bond); 154 bond_destroy(bond);
155 up_write(&bonding_rwsem);
167 rtnl_unlock(); 156 rtnl_unlock();
168 goto out; 157 goto out;
169 } 158 }
@@ -171,6 +160,8 @@ static ssize_t bonding_store_bonds(struct class *cls, const char *buffer, size_t
171 printk(KERN_ERR DRV_NAME 160 printk(KERN_ERR DRV_NAME
172 ": unable to delete non-existent bond %s\n", ifname); 161 ": unable to delete non-existent bond %s\n", ifname);
173 res = -ENODEV; 162 res = -ENODEV;
163 up_write(&bonding_rwsem);
164 rtnl_unlock();
174 goto out; 165 goto out;
175 } 166 }
176 167
@@ -183,7 +174,6 @@ err_no_cmd:
183 * get called forever, which is bad. 174 * get called forever, which is bad.
184 */ 175 */
185out: 176out:
186 up_write(&(bonding_rwsem));
187 return res; 177 return res;
188} 178}
189/* class attribute for bond_masters file. This ends up in /sys/class/net */ 179/* class attribute for bond_masters file. This ends up in /sys/class/net */
@@ -271,6 +261,9 @@ static ssize_t bonding_store_slaves(struct device *d,
271 261
272 /* Note: We can't hold bond->lock here, as bond_create grabs it. */ 262 /* Note: We can't hold bond->lock here, as bond_create grabs it. */
273 263
264 rtnl_lock();
265 down_write(&(bonding_rwsem));
266
274 sscanf(buffer, "%16s", command); /* IFNAMSIZ*/ 267 sscanf(buffer, "%16s", command); /* IFNAMSIZ*/
275 ifname = command + 1; 268 ifname = command + 1;
276 if ((strlen(command) <= 1) || 269 if ((strlen(command) <= 1) ||
@@ -336,12 +329,10 @@ static ssize_t bonding_store_slaves(struct device *d,
336 dev->mtu = bond->dev->mtu; 329 dev->mtu = bond->dev->mtu;
337 } 330 }
338 } 331 }
339 rtnl_lock();
340 res = bond_enslave(bond->dev, dev); 332 res = bond_enslave(bond->dev, dev);
341 bond_for_each_slave(bond, slave, i) 333 bond_for_each_slave(bond, slave, i)
342 if (strnicmp(slave->dev->name, ifname, IFNAMSIZ) == 0) 334 if (strnicmp(slave->dev->name, ifname, IFNAMSIZ) == 0)
343 slave->original_mtu = original_mtu; 335 slave->original_mtu = original_mtu;
344 rtnl_unlock();
345 if (res) { 336 if (res) {
346 ret = res; 337 ret = res;
347 } 338 }
@@ -359,12 +350,10 @@ static ssize_t bonding_store_slaves(struct device *d,
359 if (dev) { 350 if (dev) {
360 printk(KERN_INFO DRV_NAME ": %s: Removing slave %s\n", 351 printk(KERN_INFO DRV_NAME ": %s: Removing slave %s\n",
361 bond->dev->name, dev->name); 352 bond->dev->name, dev->name);
362 rtnl_lock();
363 if (bond->setup_by_slave) 353 if (bond->setup_by_slave)
364 res = bond_release_and_destroy(bond->dev, dev); 354 res = bond_release_and_destroy(bond->dev, dev);
365 else 355 else
366 res = bond_release(bond->dev, dev); 356 res = bond_release(bond->dev, dev);
367 rtnl_unlock();
368 if (res) { 357 if (res) {
369 ret = res; 358 ret = res;
370 goto out; 359 goto out;
@@ -389,6 +378,8 @@ err_no_cmd:
389 ret = -EPERM; 378 ret = -EPERM;
390 379
391out: 380out:
381 up_write(&(bonding_rwsem));
382 rtnl_unlock();
392 return ret; 383 return ret;
393} 384}
394 385
@@ -1423,8 +1414,6 @@ int bond_create_sysfs(void)
1423 int ret = 0; 1414 int ret = 0;
1424 struct bonding *firstbond; 1415 struct bonding *firstbond;
1425 1416
1426 init_rwsem(&bonding_rwsem);
1427
1428 /* get the netdev class pointer */ 1417 /* get the netdev class pointer */
1429 firstbond = container_of(bond_dev_list.next, struct bonding, bond_list); 1418 firstbond = container_of(bond_dev_list.next, struct bonding, bond_list);
1430 if (!firstbond) 1419 if (!firstbond)