aboutsummaryrefslogtreecommitdiffstats
path: root/net/mac80211/ieee80211.c
diff options
context:
space:
mode:
authorJohannes Berg <johannes@sipsolutions.net>2007-09-18 17:29:21 -0400
committerDavid S. Miller <davem@sunset.davemloft.net>2007-10-10 19:53:00 -0400
commit79010420cc3f78eab911598bfdd29c4b06a83e1f (patch)
treea9031164d7944f8aa90a455d297780b241f3d865 /net/mac80211/ieee80211.c
parentea49c359f36d5b40bf033c45a08332cb73777aa2 (diff)
[PATCH] mac80211: fix virtual interface locking
Florian Lohoff noticed a bug in mac80211: when bringing the master interface down while other virtual interfaces are up we call dev_close() under a spinlock which is not allowed. This patch removes the sub_if_lock used by mac80211 in favour of using an RCU list. All list manipulations are already done under rtnl so are well protected against each other, and the read-side locks we took in the RX and TX code are already in RCU read-side critical sections. Signed-off-by: Johannes Berg <johannes@sipsolutions.net> Cc: Florian Lohoff <flo@rfc822.org> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: Michal Piotrowski <michal.k.k.piotrowski@gmail.com> Cc: Satyam Sharma <satyam@infradead.org> Signed-off-by: Michael Wu <flamingice@sourmilk.net> Signed-off-by: John W. Linville <linville@tuxdriver.com>
Diffstat (limited to 'net/mac80211/ieee80211.c')
-rw-r--r--net/mac80211/ieee80211.c56
1 files changed, 25 insertions, 31 deletions
diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c
index 4e345f82f044..ccf84634f156 100644
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -93,14 +93,13 @@ static int ieee80211_master_open(struct net_device *dev)
93 struct ieee80211_sub_if_data *sdata; 93 struct ieee80211_sub_if_data *sdata;
94 int res = -EOPNOTSUPP; 94 int res = -EOPNOTSUPP;
95 95
96 read_lock(&local->sub_if_lock); 96 /* we hold the RTNL here so can safely walk the list */
97 list_for_each_entry(sdata, &local->sub_if_list, list) { 97 list_for_each_entry(sdata, &local->interfaces, list) {
98 if (sdata->dev != dev && netif_running(sdata->dev)) { 98 if (sdata->dev != dev && netif_running(sdata->dev)) {
99 res = 0; 99 res = 0;
100 break; 100 break;
101 } 101 }
102 } 102 }
103 read_unlock(&local->sub_if_lock);
104 return res; 103 return res;
105} 104}
106 105
@@ -109,11 +108,10 @@ static int ieee80211_master_stop(struct net_device *dev)
109 struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr); 108 struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
110 struct ieee80211_sub_if_data *sdata; 109 struct ieee80211_sub_if_data *sdata;
111 110
112 read_lock(&local->sub_if_lock); 111 /* we hold the RTNL here so can safely walk the list */
113 list_for_each_entry(sdata, &local->sub_if_list, list) 112 list_for_each_entry(sdata, &local->interfaces, list)
114 if (sdata->dev != dev && netif_running(sdata->dev)) 113 if (sdata->dev != dev && netif_running(sdata->dev))
115 dev_close(sdata->dev); 114 dev_close(sdata->dev);
116 read_unlock(&local->sub_if_lock);
117 115
118 return 0; 116 return 0;
119} 117}
@@ -315,8 +313,8 @@ static int ieee80211_open(struct net_device *dev)
315 313
316 sdata = IEEE80211_DEV_TO_SUB_IF(dev); 314 sdata = IEEE80211_DEV_TO_SUB_IF(dev);
317 315
318 read_lock(&local->sub_if_lock); 316 /* we hold the RTNL here so can safely walk the list */
319 list_for_each_entry(nsdata, &local->sub_if_list, list) { 317 list_for_each_entry(nsdata, &local->interfaces, list) {
320 struct net_device *ndev = nsdata->dev; 318 struct net_device *ndev = nsdata->dev;
321 319
322 if (ndev != dev && ndev != local->mdev && netif_running(ndev) && 320 if (ndev != dev && ndev != local->mdev && netif_running(ndev) &&
@@ -325,10 +323,8 @@ static int ieee80211_open(struct net_device *dev)
325 * check whether it may have the same address 323 * check whether it may have the same address
326 */ 324 */
327 if (!identical_mac_addr_allowed(sdata->type, 325 if (!identical_mac_addr_allowed(sdata->type,
328 nsdata->type)) { 326 nsdata->type))
329 read_unlock(&local->sub_if_lock);
330 return -ENOTUNIQ; 327 return -ENOTUNIQ;
331 }
332 328
333 /* 329 /*
334 * can only add VLANs to enabled APs 330 * can only add VLANs to enabled APs
@@ -339,7 +335,6 @@ static int ieee80211_open(struct net_device *dev)
339 sdata->u.vlan.ap = nsdata; 335 sdata->u.vlan.ap = nsdata;
340 } 336 }
341 } 337 }
342 read_unlock(&local->sub_if_lock);
343 338
344 switch (sdata->type) { 339 switch (sdata->type) {
345 case IEEE80211_IF_TYPE_WDS: 340 case IEEE80211_IF_TYPE_WDS:
@@ -466,14 +461,13 @@ static int ieee80211_stop(struct net_device *dev)
466 sdata->u.sta.state = IEEE80211_DISABLED; 461 sdata->u.sta.state = IEEE80211_DISABLED;
467 del_timer_sync(&sdata->u.sta.timer); 462 del_timer_sync(&sdata->u.sta.timer);
468 /* 463 /*
469 * Holding the sub_if_lock for writing here blocks 464 * When we get here, the interface is marked down.
470 * out the receive path and makes sure it's not 465 * Call synchronize_rcu() to wait for the RX path
471 * currently processing a packet that may get 466 * should it be using the interface and enqueuing
472 * added to the queue. 467 * frames at this very time on another CPU.
473 */ 468 */
474 write_lock_bh(&local->sub_if_lock); 469 synchronize_rcu();
475 skb_queue_purge(&sdata->u.sta.skb_queue); 470 skb_queue_purge(&sdata->u.sta.skb_queue);
476 write_unlock_bh(&local->sub_if_lock);
477 471
478 if (!local->ops->hw_scan && 472 if (!local->ops->hw_scan &&
479 local->scan_dev == sdata->dev) { 473 local->scan_dev == sdata->dev) {
@@ -1033,9 +1027,9 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
1033 1027
1034 rthdr->data_retries = status->retry_count; 1028 rthdr->data_retries = status->retry_count;
1035 1029
1036 read_lock(&local->sub_if_lock); 1030 rcu_read_lock();
1037 monitors = local->monitors; 1031 monitors = local->monitors;
1038 list_for_each_entry(sdata, &local->sub_if_list, list) { 1032 list_for_each_entry_rcu(sdata, &local->interfaces, list) {
1039 /* 1033 /*
1040 * Using the monitors counter is possibly racy, but 1034 * Using the monitors counter is possibly racy, but
1041 * if the value is wrong we simply either clone the skb 1035 * if the value is wrong we simply either clone the skb
@@ -1051,7 +1045,7 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
1051 continue; 1045 continue;
1052 monitors--; 1046 monitors--;
1053 if (monitors) 1047 if (monitors)
1054 skb2 = skb_clone(skb, GFP_KERNEL); 1048 skb2 = skb_clone(skb, GFP_ATOMIC);
1055 else 1049 else
1056 skb2 = NULL; 1050 skb2 = NULL;
1057 skb->dev = sdata->dev; 1051 skb->dev = sdata->dev;
@@ -1066,7 +1060,7 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
1066 } 1060 }
1067 } 1061 }
1068 out: 1062 out:
1069 read_unlock(&local->sub_if_lock); 1063 rcu_read_unlock();
1070 if (skb) 1064 if (skb)
1071 dev_kfree_skb(skb); 1065 dev_kfree_skb(skb);
1072} 1066}
@@ -1154,8 +1148,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
1154 1148
1155 INIT_LIST_HEAD(&local->modes_list); 1149 INIT_LIST_HEAD(&local->modes_list);
1156 1150
1157 rwlock_init(&local->sub_if_lock); 1151 INIT_LIST_HEAD(&local->interfaces);
1158 INIT_LIST_HEAD(&local->sub_if_list);
1159 1152
1160 INIT_DELAYED_WORK(&local->scan_work, ieee80211_sta_scan_work); 1153 INIT_DELAYED_WORK(&local->scan_work, ieee80211_sta_scan_work);
1161 ieee80211_rx_bss_list_init(mdev); 1154 ieee80211_rx_bss_list_init(mdev);
@@ -1175,7 +1168,8 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
1175 sdata->u.ap.force_unicast_rateidx = -1; 1168 sdata->u.ap.force_unicast_rateidx = -1;
1176 sdata->u.ap.max_ratectrl_rateidx = -1; 1169 sdata->u.ap.max_ratectrl_rateidx = -1;
1177 ieee80211_if_sdata_init(sdata); 1170 ieee80211_if_sdata_init(sdata);
1178 list_add_tail(&sdata->list, &local->sub_if_list); 1171 /* no RCU needed since we're still during init phase */
1172 list_add_tail(&sdata->list, &local->interfaces);
1179 1173
1180 tasklet_init(&local->tx_pending_tasklet, ieee80211_tx_pending, 1174 tasklet_init(&local->tx_pending_tasklet, ieee80211_tx_pending,
1181 (unsigned long)local); 1175 (unsigned long)local);
@@ -1334,7 +1328,6 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
1334{ 1328{
1335 struct ieee80211_local *local = hw_to_local(hw); 1329 struct ieee80211_local *local = hw_to_local(hw);
1336 struct ieee80211_sub_if_data *sdata, *tmp; 1330 struct ieee80211_sub_if_data *sdata, *tmp;
1337 struct list_head tmp_list;
1338 int i; 1331 int i;
1339 1332
1340 tasklet_kill(&local->tx_pending_tasklet); 1333 tasklet_kill(&local->tx_pending_tasklet);
@@ -1348,11 +1341,12 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
1348 if (local->apdev) 1341 if (local->apdev)
1349 ieee80211_if_del_mgmt(local); 1342 ieee80211_if_del_mgmt(local);
1350 1343
1351 write_lock_bh(&local->sub_if_lock); 1344 /*
1352 list_replace_init(&local->sub_if_list, &tmp_list); 1345 * At this point, interface list manipulations are fine
1353 write_unlock_bh(&local->sub_if_lock); 1346 * because the driver cannot be handing us frames any
1354 1347 * more and the tasklet is killed.
1355 list_for_each_entry_safe(sdata, tmp, &tmp_list, list) 1348 */
1349 list_for_each_entry_safe(sdata, tmp, &local->interfaces, list)
1356 __ieee80211_if_del(local, sdata); 1350 __ieee80211_if_del(local, sdata);
1357 1351
1358 rtnl_unlock(); 1352 rtnl_unlock();