rx racing against removing interfaces?

rx racing against removing interfaces?

[Johannes Berg]

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

Isn't there a race there when you remove interfaces and at the same time
__ieee80211_rx is running? I don't see anything that should stop that,
and if it happens we'll probably blow up pretty spectacularly with
accesses to a freed netdev, or even sending it frames...

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: rx racing against removing interfaces?

[Michael Buesch]

On Tuesday 27 March 2007 01:05, Johannes Berg wrote:
> Isn't there a race there when you remove interfaces and at the same time
> __ieee80211_rx is running? I don't see anything that should stop that,
> and if it happens we'll probably blow up pretty spectacularly with
> accesses to a freed netdev, or even sending it frames...

Well, IMHO __ieee80211_rx should be internal to mac80211 and not be exported.
(Same goes for the IRQ-unsafe txstatus report). We can't (and we really don't
want to) call synchronously into the stack from any driver. We want to
defer the packet processing work to a tasklet. So we always want to
call the _irqsafe functions. Why do we? Because of locking issues.
1) There's no need to hold the lowlevel driver IRQ spinlock while
processing the packet in the upper layers. We want to release it. Easiest
and non-racy way to do this is to use the _irqsafe function and its tasklet.
2) We want to prevent recursion back into the driver through a callback,
which would result in lock recursion (we had these problems in softmac, as
it is synchronous in most rx parts).

So why do I tell you this?
I think there's no way we can call __ieee80211_rx after removing the
interface, as the tasklet should be stopped first. But someone should
verify if I'm right here.
But anyway, the driver shouldn't be able to call __ieee80211_rx after the
device is down, too, as the driver has to make sure IRQs are stopped
when the device is brought down.

So we have kind of implicit locking here, but I think it's ok.

-- 
Greetings Michael.

Re: rx racing against removing interfaces?

[Michael Buesch]

On Wednesday 28 March 2007 12:52, Michael Buesch wrote:
> On Tuesday 27 March 2007 01:05, Johannes Berg wrote:
> > Isn't there a race there when you remove interfaces and at the same time
> > __ieee80211_rx is running? I don't see anything that should stop that,
> > and if it happens we'll probably blow up pretty spectacularly with
> > accesses to a freed netdev, or even sending it frames...
> 
> Well, IMHO __ieee80211_rx should be internal to mac80211 and not be exported.
> (Same goes for the IRQ-unsafe txstatus report). We can't (and we really don't
> want to) call synchronously into the stack from any driver. We want to
> defer the packet processing work to a tasklet. So we always want to
> call the _irqsafe functions. Why do we? Because of locking issues.
> 1) There's no need to hold the lowlevel driver IRQ spinlock while
> processing the packet in the upper layers. We want to release it. Easiest
> and non-racy way to do this is to use the _irqsafe function and its tasklet.
> 2) We want to prevent recursion back into the driver through a callback,
> which would result in lock recursion (we had these problems in softmac, as
> it is synchronous in most rx parts).
> 
> So why do I tell you this?
> I think there's no way we can call __ieee80211_rx after removing the
> interface, as the tasklet should be stopped first. But someone should
> verify if I'm right here.
> But anyway, the driver shouldn't be able to call __ieee80211_rx after the
> device is down, too, as the driver has to make sure IRQs are stopped
> when the device is brought down.
> 
> So we have kind of implicit locking here, but I think it's ok.


Oh, you mean virtual interfaces.. yeah, there's probably a race. I don't
know the locking there.
But my comment about the non-irqsafe-variants-should-be-hidden still applies :)

-- 
Greetings Michael.

Re: rx racing against removing interfaces?

[Johannes Berg]

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

On Wed, 2007-03-28 at 12:59 +0200, Michael Buesch wrote:

> Oh, you mean virtual interfaces.. yeah, there's probably a race. I don't
> know the locking there.

Yes, I mean virtual interfaces, when the thing is in progress while we
iterate through.

> But my comment about the non-irqsafe-variants-should-be-hidden still applies :)

I'll reread it ;)

But I'm not sure I fully agree. If the driver calls __ieee80211_rx in
softirq context that should be good without requiring to reschedule in
mac80211.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: rx racing against removing interfaces?

[Michael Buesch]

On Wednesday 28 March 2007 13:07, Johannes Berg wrote:
> On Wed, 2007-03-28 at 12:59 +0200, Michael Buesch wrote:
> 
> > Oh, you mean virtual interfaces.. yeah, there's probably a race. I don't
> > know the locking there.
> 
> Yes, I mean virtual interfaces, when the thing is in progress while we
> iterate through.
> 
> > But my comment about the non-irqsafe-variants-should-be-hidden still applies :)
> 
> I'll reread it ;)
> 
> But I'm not sure I fully agree. If the driver calls __ieee80211_rx in
> softirq context that should be good without requiring to reschedule in
> mac80211.

I'm not sure there is an easy and non-racy way to drop the driver IRQ lock
while calling __ieee80211_rx synchronously.

-- 
Greetings Michael.

Re: rx racing against removing interfaces?

[Johannes Berg]

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

On Wed, 2007-03-28 at 13:32 +0200, Michael Buesch wrote:
> On Wednesday 28 March 2007 13:07, Johannes Berg wrote:
> > On Wed, 2007-03-28 at 12:59 +0200, Michael Buesch wrote:
> > 
> > > Oh, you mean virtual interfaces.. yeah, there's probably a race. I don't
> > > know the locking there.
> > 
> > Yes, I mean virtual interfaces, when the thing is in progress while we
> > iterate through.
> > 
> > > But my comment about the non-irqsafe-variants-should-be-hidden still applies :)
> > 
> > I'll reread it ;)
> > 
> > But I'm not sure I fully agree. If the driver calls __ieee80211_rx in
> > softirq context that should be good without requiring to reschedule in
> > mac80211.
> 
> I'm not sure there is an easy and non-racy way to drop the driver IRQ lock
> while calling __ieee80211_rx synchronously.

Depends on the driver, I guess. rt* seem to reschedule already and call
__ieee80211_rx.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: rx racing against removing interfaces?

[Jiri Benc]

On Tue, 27 Mar 2007 01:05:22 +0200, Johannes Berg wrote:
> Isn't there a race there when you remove interfaces and at the same time
> __ieee80211_rx is running? I don't see anything that should stop that,
> and if it happens we'll probably blow up pretty spectacularly with
> accesses to a freed netdev, or even sending it frames...

Yes, there is a race.

- sta_info should be holding a reference to a net_device in its dev
  field (sta_info_add).
- walking through the local->sub_if_list in __ieee80211_rx should
  happen under a lock
- while invoking rx handlers in the list_for_each_entry loop (they
  shouldn't be called under the lock above - hm, another thing that
  makes locking in mac80211 hard) we should hold a reference to the
  appropriate net_device

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: rx racing against removing interfaces?

[Michael Wu]

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

On Wednesday 18 April 2007 14:31, Jiri Benc wrote:
> Yes, there is a race.
>
> - sta_info should be holding a reference to a net_device in its dev
>   field (sta_info_add).
> - walking through the local->sub_if_list in __ieee80211_rx should
>   happen under a lock
> - while invoking rx handlers in the list_for_each_entry loop (they
>   shouldn't be called under the lock above - hm, another thing that
>   makes locking in mac80211 hard) we should hold a reference to the
>   appropriate net_device
This should be all fixed in the attached patch. It applies on top of johill's 
no-rtnl-in-wext patch (because RTNLs are eliminated) and my radiotap RX patch 
(just because I have it in my tree).

Unfortunately, it introduces a deadlock in the wext interface for removing 
virtual interfaces, which is why this isn't signed off. I should be able to 
figure that one out..

-Michael Wu

[-- Attachment #1.2: fix-subif-locking.diff --]
[-- Type: text/x-diff, Size: 30530 bytes --]

mac80211: fix virtual interface related locking

From: Michael Wu <flamingice@sourmilk.net>

This converts sub_if_lock to a rw lock and makes all code touching
sub_if_list use it, grabs mdev's tx lock in set_multicast_list to
synchronize multicast configuration, and eliminates all explicit rtnl
locking.
---

 net/mac80211/debugfs.c         |   15 ------
 net/mac80211/ieee80211.c       |   72 ++++++++++++++++--------------
 net/mac80211/ieee80211_cfg.c   |   36 +++------------
 net/mac80211/ieee80211_i.h     |    7 +--
 net/mac80211/ieee80211_iface.c |   95 ++++++++++++++--------------------------
 net/mac80211/ieee80211_ioctl.c |   68 ++++++++++++++++-------------
 net/mac80211/ieee80211_sta.c   |   24 ++--------
 7 files changed, 124 insertions(+), 193 deletions(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index fbb003e..1a124c5 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -118,16 +118,6 @@ DEBUGFS_READONLY_FILE(rate_ctrl_alg, 100, "%s",
 
 /* statistics stuff */
 
-static inline int rtnl_lock_local(struct ieee80211_local *local)
-{
-	rtnl_lock();
-	if (unlikely(local->reg_state != IEEE80211_DEV_REGISTERED)) {
-		rtnl_unlock();
-		return -ENODEV;
-	}
-	return 0;
-}
-
 #define DEBUGFS_STATS_FILE(name, buflen, fmt, value...)			\
 	DEBUGFS_READONLY_FILE(stats_ ##name, buflen, fmt, ##value)
 
@@ -144,12 +134,7 @@ static ssize_t format_devstat_counter(struct ieee80211_local *local,
 	if (!local->ops->get_stats)
 		return -EOPNOTSUPP;
 
-	res = rtnl_lock_local(local);
-	if (res)
-		return res;
-
 	res = local->ops->get_stats(local_to_hw(local), &stats);
-	rtnl_unlock();
 	if (!res)
 		res = printvalue(&stats, buf, sizeof(buf));
 	return simple_read_from_buffer(userbuf, count, ppos, buf, res);
diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c
index ad84eef..fae3666 100644
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -953,7 +953,7 @@ static void purge_old_ps_buffers(struct ieee80211_local *local)
 	struct ieee80211_sub_if_data *sdata;
 	struct sta_info *sta;
 
-	spin_lock_bh(&local->sub_if_lock);
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
 		struct ieee80211_if_ap *ap;
 		if (sdata->dev == local->mdev ||
@@ -967,7 +967,7 @@ static void purge_old_ps_buffers(struct ieee80211_local *local)
 		}
 		total += skb_queue_len(&ap->ps_bc_buf);
 	}
-	spin_unlock_bh(&local->sub_if_lock);
+	read_unlock(&local->sub_if_lock);
 
 	spin_lock_bh(&local->sta_lock);
 	list_for_each_entry(sta, &local->sta_list, list) {
@@ -2148,6 +2148,7 @@ static void ieee80211_set_multicast_list(struct net_device *dev)
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	unsigned short flags;
 
+	netif_tx_lock(local->mdev);
 	if (((dev->flags & IFF_ALLMULTI) != 0) ^ (sdata->allmulti != 0)) {
 		if (sdata->allmulti) {
 			sdata->allmulti = 0;
@@ -2177,9 +2178,12 @@ static void ieee80211_set_multicast_list(struct net_device *dev)
 			flags |= IFF_ALLMULTI;
 		if (local->iff_promiscs)
 			flags |= IFF_PROMISC;
+		read_lock(&local->sub_if_lock);
 		local->ops->set_multicast_list(local_to_hw(local), flags,
 					      local->mc_count);
+		read_unlock(&local->sub_if_lock);
 	}
+	netif_tx_unlock(local->mdev);
 }
 
 struct dev_mc_list *ieee80211_get_mc_list_item(struct ieee80211_hw *hw,
@@ -2220,12 +2224,11 @@ static struct net_device_stats *ieee80211_get_stats(struct net_device *dev)
 	return &(sdata->stats);
 }
 
-void ieee80211_if_shutdown(struct net_device *dev)
+static void ieee80211_if_shutdown(struct net_device *dev)
 {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 
-	ASSERT_RTNL();
 	switch (sdata->type) {
 	case IEEE80211_IF_TYPE_STA:
 	case IEEE80211_IF_TYPE_IBSS:
@@ -2269,6 +2272,7 @@ static int ieee80211_master_open(struct net_device *dev)
 	struct ieee80211_sub_if_data *sdata;
 	int res = -EOPNOTSUPP;
 
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
 		if (sdata->dev != dev && netif_running(sdata->dev)) {
 			res = 0;
@@ -2276,6 +2280,7 @@ static int ieee80211_master_open(struct net_device *dev)
 			break;
 		}
 	}
+	read_unlock(&local->sub_if_lock);
 	return res;
 }
 
@@ -2285,10 +2290,14 @@ static int ieee80211_master_stop(struct net_device *dev)
 	struct ieee80211_sub_if_data *sdata;
 
 	tasklet_disable(&local->tx_pending_tasklet);
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
-		if (sdata->dev != dev && netif_running(sdata->dev))
+		if (sdata->dev != dev && netif_running(sdata->dev)) {
+			read_unlock(&local->sub_if_lock);
 			return -EOPNOTSUPP;
+		}
 	}
+	read_unlock(&local->sub_if_lock);
 	return 0;
 }
 
@@ -2346,14 +2355,18 @@ static int ieee80211_open(struct net_device *dev)
 	int res;
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(nsdata, &local->sub_if_list, list) {
 		struct net_device *ndev = nsdata->dev;
 
 		if (ndev != dev && ndev != local->mdev && netif_running(ndev) &&
 		    compare_ether_addr(dev->dev_addr, ndev->dev_addr) == 0 &&
-		    !identical_mac_addr_allowed(sdata->type, nsdata->type))
+		    !identical_mac_addr_allowed(sdata->type, nsdata->type)) {
+			read_unlock(&local->sub_if_lock);
 			return -ENOTUNIQ;
+		}
 	}
+	read_unlock(&local->sub_if_lock);
 
 	if (sdata->type == IEEE80211_IF_TYPE_WDS &&
 	    is_zero_ether_addr(sdata->u.wds.remote_addr))
@@ -4004,6 +4017,7 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb,
 		struct sk_buff *skb_new;
 		u8 *bssid = ieee80211_get_bssid(hdr, skb->len - radiotap_len);
 
+		read_lock(&local->sub_if_lock);
 		list_for_each_entry(sdata, &local->sub_if_list, list) {
 			rx.u.rx.ra_match = 1;
 			switch (sdata->type) {
@@ -4039,10 +4053,9 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb,
 					rx.u.rx.ra_match = 0;
 				} else if (!sta)
 					sta = rx.sta =
-						ieee80211_ibss_add_sta(local->mdev,
+						ieee80211_ibss_add_sta(sdata->dev,
 								       skb, bssid,
 								       hdr->addr2);
-						/* FIXME: call with sdata->dev */
 				break;
 			case IEEE80211_IF_TYPE_AP:
 				if (!bssid) {
@@ -4097,6 +4110,7 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb,
 						     &rx, sta);
 		} else
 			dev_kfree_skb(skb);
+		read_unlock(&local->sub_if_lock);
 	}
 
   end:
@@ -4241,11 +4255,13 @@ static void ieee80211_stat_refresh(unsigned long data)
 	spin_unlock_bh(&local->sta_lock);
 
 	/* go through all subinterfaces */
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
 		sdata->channel_use = (sdata->channel_use_raw /
 				      local->stat_time) / CHAN_UTIL_PER_10MS;
 		sdata->channel_use_raw = 0;
 	}
+	read_unlock(&local->sub_if_lock);
 
 	/* hardware interface */
 	local->channel_use = (local->channel_use_raw /
@@ -4683,7 +4699,6 @@ int ieee80211_init_rate_ctrl_alg(struct ieee80211_local *local,
 {
 	struct rate_control_ref *ref, *old;
 
-	ASSERT_RTNL();
 	if (local->open_count || netif_running(local->mdev) ||
 	    (local->apdev && netif_running(local->apdev)))
 		return -EBUSY;
@@ -4799,7 +4814,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 
 	INIT_LIST_HEAD(&local->modes_list);
 
-	spin_lock_init(&local->sub_if_lock);
+	rwlock_init(&local->sub_if_lock);
 	INIT_LIST_HEAD(&local->sub_if_list);
 
 	INIT_DELAYED_WORK(&local->scan_work, ieee80211_sta_scan_work);
@@ -4841,7 +4856,6 @@ EXPORT_SYMBOL(ieee80211_alloc_hw);
 int ieee80211_register_hw(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
-	struct net_device *sta_dev;
 	int result;
 
 	result = wiphy_register(local->hw.wiphy);
@@ -4865,17 +4879,10 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 	if (result < 0)
 		goto fail_sta_info;
 
-	rtnl_lock();
-	result = dev_alloc_name(local->mdev, local->mdev->name);
-	if (result < 0) {
-		rtnl_unlock();
-		goto fail_dev;
-	}
-
 	memcpy(local->mdev->dev_addr, local->hw.wiphy->perm_addr, ETH_ALEN);
 	SET_NETDEV_DEV(local->mdev, wiphy_dev(local->hw.wiphy));
 
-	result = register_netdevice(local->mdev);
+	result = register_netdev(local->mdev);
 	if (result < 0)
 		goto fail_dev;
 
@@ -4896,16 +4903,16 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 		goto fail_wep;
 	}
 
-	/* TODO: add rtnl locking around device creation and qdisc install */
 	ieee80211_install_qdisc(local->mdev);
 
 	/* add one default STA interface */
-	result = ieee80211_if_add(local->mdev, "wlan%d", 1, &sta_dev);
-	if (result == 0)
-		ieee80211_if_set_type(sta_dev, IEEE80211_IF_TYPE_STA);
+	result = ieee80211_if_add(local->mdev, "wlan%d", NULL,
+				  IEEE80211_IF_TYPE_STA);
+	if (result)
+		printk(KERN_WARNING "%s: Failed to add default virtual iface\n",
+		       local->mdev->name);
 
 	local->reg_state = IEEE80211_DEV_REGISTERED;
-	rtnl_unlock();
 
 	ieee80211_led_init(local);
 
@@ -4915,9 +4922,8 @@ fail_wep:
 	rate_control_deinitialize(local);
 fail_rate:
 	ieee80211_debugfs_remove_netdev(IEEE80211_DEV_TO_SUB_IF(local->mdev));
-	unregister_netdevice(local->mdev);
+	unregister_netdev(local->mdev);
 fail_dev:
-	rtnl_unlock();
 	sta_info_stop(local);
 fail_sta_info:
 	debugfs_hw_del(local);
@@ -4962,23 +4968,24 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
 	struct ieee80211_sub_if_data *sdata, *tmp;
+	LIST_HEAD(tmp_list);
 	int i;
 
 	tasklet_kill(&local->tx_pending_tasklet);
 	tasklet_kill(&local->tasklet);
 
-	rtnl_lock();
-
 	BUG_ON(local->reg_state != IEEE80211_DEV_REGISTERED);
 
 	local->reg_state = IEEE80211_DEV_UNREGISTERED;
 	if (local->apdev)
 		ieee80211_if_del_mgmt(local);
 
-	list_for_each_entry_safe(sdata, tmp, &local->sub_if_list, list)
-		__ieee80211_if_del(local, sdata);
+	write_lock_bh(&local->sub_if_lock);
+	list_replace_init(&local->sub_if_list, &tmp_list);
+	write_unlock_bh(&local->sub_if_lock);
 
-	rtnl_unlock();
+	list_for_each_entry_safe(sdata, tmp, &tmp_list, list)
+		__ieee80211_if_del(local, sdata);
 
 	if (local->stat_time)
 		del_timer_sync(&local->stat_timer);
@@ -4988,8 +4995,7 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
 		flush_scheduled_work();
 		/* The scan_work is guaranteed not to be called at this
 		 * point. It is not scheduled and not running now. It can be
-		 * scheduled again only by sta_work (stopped by now) or under
-		 * rtnl lock. */
+		 * scheduled again only by sta_work (stopped by now). */
 	}
 
 	ieee80211_rx_bss_list_deinit(local->mdev);
diff --git a/net/mac80211/ieee80211_cfg.c b/net/mac80211/ieee80211_cfg.c
index 11cfcf5..711754e 100644
--- a/net/mac80211/ieee80211_cfg.c
+++ b/net/mac80211/ieee80211_cfg.c
@@ -12,24 +12,11 @@
 #include "ieee80211_i.h"
 #include "ieee80211_cfg.h"
 
-/* copied from ieee80211_sysfs.c for now ... */
-static inline int rtnl_lock_local(struct ieee80211_local *local)
-{
-	rtnl_lock();
-	if (unlikely(local->reg_state != IEEE80211_DEV_REGISTERED)) {
-		rtnl_unlock();
-		return -ENODEV;
-	}
-	return 0;
-}
-
-
 static int ieee80211_add_iface(struct wiphy *wiphy, char *name,
 			       unsigned int type)
 {
 	struct ieee80211_local *local = wiphy_priv(wiphy);
-	struct net_device *new_dev;
-	int res, itype;
+	int itype;
 
 	switch (type) {
 	case NL80211_IFTYPE_UNSPECIFIED:
@@ -54,34 +41,23 @@ static int ieee80211_add_iface(struct wiphy *wiphy, char *name,
 		return -EINVAL;
 	}
 
-	res = rtnl_lock_local(local);
-	if (res)
-		return res;
-
-	res = ieee80211_if_add(local->mdev, name, 0, &new_dev);
-	if (res == 0)
-		ieee80211_if_set_type(new_dev, itype);
-	rtnl_unlock();
-	return res;
+	return ieee80211_if_add(local->mdev, name, NULL, itype);
 }
 
 static int ieee80211_del_iface(struct wiphy *wiphy, int ifindex)
 {
 	struct ieee80211_local *local = wiphy_priv(wiphy);
-	int res;
 	struct net_device *dev;
 	char *name;
 
-	res = rtnl_lock_local(local);
-	if (res)
-		return res;
 	dev = dev_get_by_index(ifindex);
+	if (!dev)
+		return 0;
+
 	name = dev->name;
 	dev_put(dev);
 
-	res = ieee80211_if_remove(local->mdev, name, -1);
-	rtnl_unlock();
-	return res;
+	return ieee80211_if_remove(local->mdev, name, -1);
 }
 
 struct cfg80211_ops mac80211_config_ops = {
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a756b18..2bf26cf 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -495,7 +495,7 @@ struct ieee80211_local {
 	ieee80211_rx_handler *rx_handlers;
 	ieee80211_tx_handler *tx_handlers;
 
-	spinlock_t sub_if_lock; /* mutex for STA data structures */
+	rwlock_t sub_if_lock; /* protects sub_if_list */
 	struct list_head sub_if_list;
 	int sta_scanning;
 	int scan_channel_idx;
@@ -758,7 +758,6 @@ void ieee80211_tx_set_iswep(struct ieee80211_txrx_data *tx);
 int ieee80211_if_update_wds(struct net_device *dev, u8 *remote_addr);
 void ieee80211_if_setup(struct net_device *dev);
 void ieee80211_if_mgmt_setup(struct net_device *dev);
-void ieee80211_if_shutdown(struct net_device *dev);
 int ieee80211_init_rate_ctrl_alg(struct ieee80211_local *local,
 				 const char *name);
 struct net_device_stats *ieee80211_dev_stats(struct net_device *dev);
@@ -835,15 +834,13 @@ int ieee80211_sta_disassociate(struct net_device *dev, u16 reason);
 
 /* ieee80211_iface.c */
 int ieee80211_if_add(struct net_device *dev, const char *name,
-		     int format, struct net_device **new_dev);
+		     struct net_device **new_dev, int type);
 void ieee80211_if_set_type(struct net_device *dev, int type);
 void ieee80211_if_reinit(struct net_device *dev);
 void __ieee80211_if_del(struct ieee80211_local *local,
 			struct ieee80211_sub_if_data *sdata);
-void ieee80211_if_del(struct net_device *dev);
 int ieee80211_if_remove(struct net_device *dev, const char *name, int id);
 void ieee80211_if_free(struct net_device *dev);
-void ieee80211_if_flush(struct net_device *dev);
 void ieee80211_if_sdata_init(struct ieee80211_sub_if_data *sdata);
 int ieee80211_if_add_mgmt(struct ieee80211_local *local);
 void ieee80211_if_del_mgmt(struct ieee80211_local *local);
diff --git a/net/mac80211/ieee80211_iface.c b/net/mac80211/ieee80211_iface.c
index c97b45c..96346b9 100644
--- a/net/mac80211/ieee80211_iface.c
+++ b/net/mac80211/ieee80211_iface.c
@@ -36,33 +36,19 @@ static void ieee80211_if_sdata_deinit(struct ieee80211_sub_if_data *sdata)
 	}
 }
 
-/* Must be called with rtnl lock held. */
 int ieee80211_if_add(struct net_device *dev, const char *name,
-		     int format, struct net_device **new_dev)
+		     struct net_device **new_dev, int type)
 {
 	struct net_device *ndev;
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_sub_if_data *sdata = NULL;
 	int ret;
 
-	ASSERT_RTNL();
-	ndev = *new_dev = alloc_netdev(sizeof(struct ieee80211_sub_if_data),
-				       "", ieee80211_if_setup);
+	ndev = alloc_netdev(sizeof(struct ieee80211_sub_if_data),
+			    name, ieee80211_if_setup);
 	if (!ndev)
 		return -ENOMEM;
 
-	if (*name == '\0') {
-		snprintf(ndev->name, IFNAMSIZ, "%s.%%d", dev->name);
-		format = 1;
-	}
-
-	if (format) {
-		ret = dev_alloc_name(ndev, name);
-		if (ret < 0)
-			goto fail;
-	} else
-		snprintf(ndev->name, IFNAMSIZ, "%s", name);
-
 	memcpy(ndev->dev_addr, local->hw.wiphy->perm_addr, ETH_ALEN);
 	ndev->base_addr = dev->base_addr;
 	ndev->irq = dev->irq;
@@ -78,19 +64,32 @@ int ieee80211_if_add(struct net_device *dev, const char *name,
 	sdata->local = local;
 	ieee80211_if_sdata_init(sdata);
 
-	ret = register_netdevice(ndev);
+	ret = register_netdev(ndev);
 	if (ret)
 		goto fail;
 
 	ieee80211_debugfs_add_netdev(sdata);
+	ieee80211_if_set_type(ndev, type);
+
+	write_lock_bh(&local->sub_if_lock);
+	if (unlikely(local->reg_state == IEEE80211_DEV_UNREGISTERED)) {
+		write_unlock_bh(&local->sub_if_lock);
+		__ieee80211_if_del(local, sdata);
+		return -ENODEV;
+	}
 	list_add(&sdata->list, &local->sub_if_list);
+	if (new_dev) {
+		dev_hold(ndev);
+		*new_dev = ndev;
+	}
+	write_unlock_bh(&local->sub_if_lock);
+
 	ieee80211_update_default_wep_only(local);
 
 	return 0;
 
 fail:
 	free_netdev(ndev);
-	*new_dev = NULL;
 	return ret;
 }
 
@@ -100,15 +99,10 @@ int ieee80211_if_add_mgmt(struct ieee80211_local *local)
 	struct ieee80211_sub_if_data *nsdata;
 	int ret;
 
-	ASSERT_RTNL();
-
-	ndev = alloc_netdev(sizeof(struct ieee80211_sub_if_data), "",
+	ndev = alloc_netdev(sizeof(struct ieee80211_sub_if_data), "wmgmt%d",
 			    ieee80211_if_mgmt_setup);
 	if (!ndev)
 		return -ENOMEM;
-	ret = dev_alloc_name(ndev, "wmgmt%d");
-	if (ret < 0)
-		goto fail;
 
 	memcpy(ndev->dev_addr, local->hw.wiphy->perm_addr, ETH_ALEN);
 	SET_NETDEV_DEV(ndev, wiphy_dev(local->hw.wiphy));
@@ -121,7 +115,7 @@ int ieee80211_if_add_mgmt(struct ieee80211_local *local)
 	nsdata->local = local;
 	ieee80211_if_sdata_init(nsdata);
 
-	ret = register_netdevice(ndev);
+	ret = register_netdev(ndev);
 	if (ret)
 		goto fail;
 
@@ -141,11 +135,10 @@ void ieee80211_if_del_mgmt(struct ieee80211_local *local)
 {
 	struct net_device *apdev;
 
-	ASSERT_RTNL();
 	apdev = local->apdev;
 	ieee80211_debugfs_remove_netdev(IEEE80211_DEV_TO_SUB_IF(apdev));
 	local->apdev = NULL;
-	unregister_netdevice(apdev);
+	unregister_netdev(apdev);
 }
 
 void ieee80211_if_set_type(struct net_device *dev, int type)
@@ -203,7 +196,6 @@ void ieee80211_if_set_type(struct net_device *dev, int type)
 	ieee80211_update_default_wep_only(local);
 }
 
-/* Must be called with rtnl lock held. */
 void ieee80211_if_reinit(struct net_device *dev)
 {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
@@ -211,7 +203,6 @@ void ieee80211_if_reinit(struct net_device *dev)
 	struct sta_info *sta;
 	int i;
 
-	ASSERT_RTNL();
 	ieee80211_if_sdata_deinit(sdata);
 	for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
 		if (!sdata->keys[i])
@@ -233,16 +224,22 @@ void ieee80211_if_reinit(struct net_device *dev)
 		/* Remove all virtual interfaces that use this BSS
 		 * as their sdata->bss */
 		struct ieee80211_sub_if_data *tsdata, *n;
+		LIST_HEAD(tmp_list);
 
+		write_lock_bh(&local->sub_if_lock);
 		list_for_each_entry_safe(tsdata, n, &local->sub_if_list, list) {
 			if (tsdata != sdata && tsdata->bss == &sdata->u.ap) {
 				printk(KERN_DEBUG "%s: removing virtual "
 				       "interface %s because its BSS interface"
 				       " is being removed\n",
 				       sdata->dev->name, tsdata->dev->name);
-				__ieee80211_if_del(local, tsdata);
+				list_move_tail(&tsdata->list, &tmp_list);
 			}
 		}
+		write_unlock_bh(&local->sub_if_lock);
+
+		list_for_each_entry_safe(tsdata, n, &tmp_list, list)
+			__ieee80211_if_del(local, tsdata);
 
 		kfree(sdata->u.ap.beacon_head);
 		kfree(sdata->u.ap.beacon_tail);
@@ -296,36 +293,35 @@ void ieee80211_if_reinit(struct net_device *dev)
 	ieee80211_if_sdata_init(sdata);
 }
 
-/* Must be called with rtnl lock held. */
 void __ieee80211_if_del(struct ieee80211_local *local,
 			struct ieee80211_sub_if_data *sdata)
 {
 	struct net_device *dev = sdata->dev;
 
-	list_del(&sdata->list);
 	ieee80211_debugfs_remove_netdev(sdata);
-	unregister_netdevice(dev);
+	unregister_netdev(dev);
 	/* Except master interface, the net_device will be freed by
 	 * net_device->destructor (i. e. ieee80211_if_free). */
 }
 
-/* Must be called with rtnl lock held. */
 int ieee80211_if_remove(struct net_device *dev, const char *name, int id)
 {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_sub_if_data *sdata, *n;
 
-	ASSERT_RTNL();
-
+	write_lock_bh(&local->sub_if_lock);
 	list_for_each_entry_safe(sdata, n, &local->sub_if_list, list) {
 		if ((sdata->type == id || id == -1) &&
 		    strcmp(name, sdata->dev->name) == 0 &&
 		    sdata->dev != local->mdev) {
+			list_del(&sdata->list);
+			write_unlock_bh(&local->sub_if_lock);
 			__ieee80211_if_del(local, sdata);
 			ieee80211_update_default_wep_only(local);
 			return 0;
 		}
 	}
+	write_unlock_bh(&local->sub_if_lock);
 	return -ENODEV;
 }
 
@@ -339,28 +335,3 @@ void ieee80211_if_free(struct net_device *dev)
 	ieee80211_if_sdata_deinit(sdata);
 	free_netdev(dev);
 }
-
-/* Must be called with rtnl lock held. */
-void ieee80211_if_flush(struct net_device *dev)
-{
-	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
-	struct ieee80211_sub_if_data *sdata, *n;
-
-	ASSERT_RTNL();
-	list_for_each_entry_safe(sdata, n, &local->sub_if_list, list) {
-		__ieee80211_if_del(local, sdata);
-	}
-}
-
-void ieee80211_if_del(struct net_device *dev)
-{
-	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
-	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-
-	rtnl_lock();
-	if (sdata->type == IEEE80211_IF_TYPE_MGMT)
-		ieee80211_if_del_mgmt(local);
-	else
-		__ieee80211_if_del(local, sdata);
-	rtnl_unlock();
-}
diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index 2ff762d..17dbf05 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -1003,23 +1003,32 @@ static int ieee80211_ioctl_add_if(struct net_device *dev,
 		if (left < sizeof(struct hostapd_if_wds))
 			return -EPROTO;
 
-		res = ieee80211_if_add(dev, param->u.if_info.name, 0, &new_dev);
+		res = ieee80211_if_add(dev, param->u.if_info.name, &new_dev,
+				       IEEE80211_IF_TYPE_WDS);
 		if (res)
 			return res;
-		ieee80211_if_set_type(new_dev, IEEE80211_IF_TYPE_WDS);
 		res = ieee80211_if_update_wds(new_dev, wds->remote_addr);
-		if (res)
-			__ieee80211_if_del(wdev_priv(dev->ieee80211_ptr),
-					   IEEE80211_DEV_TO_SUB_IF(new_dev));
+		if (unlikely(res)) {
+			struct ieee80211_local *local =
+				wdev_priv(dev->ieee80211_ptr);
+			struct ieee80211_sub_if_data *sdata = 
+				IEEE80211_DEV_TO_SUB_IF(new_dev);
+			write_lock_bh(&local->sub_if_lock);
+			list_del(&sdata->list);
+			dev_put(new_dev);
+			write_unlock_bh(&local->sub_if_lock);
+			__ieee80211_if_del(local, sdata);
+		} else
+			dev_put(new_dev);
 		return res;
 	case HOSTAP_IF_VLAN:
 		if (left < sizeof(struct hostapd_if_vlan))
 			return -EPROTO;
 
-		res = ieee80211_if_add(dev, param->u.if_info.name, 0, &new_dev);
+		res = ieee80211_if_add(dev, param->u.if_info.name, NULL,
+				       IEEE80211_IF_TYPE_VLAN);
 		if (res)
 			return res;
-		ieee80211_if_set_type(new_dev, IEEE80211_IF_TYPE_VLAN);
 #if 0
 		res = ieee80211_if_update_vlan(new_dev, vlan->id);
 		if (res)
@@ -1033,21 +1042,22 @@ static int ieee80211_ioctl_add_if(struct net_device *dev,
 		if (left < sizeof(struct hostapd_if_bss))
 			return -EPROTO;
 
-		res = ieee80211_if_add(dev, param->u.if_info.name, 0, &new_dev);
+		res = ieee80211_if_add(dev, param->u.if_info.name, &new_dev,
+				       IEEE80211_IF_TYPE_AP);
 		if (res)
 			return res;
-		ieee80211_if_set_type(new_dev, IEEE80211_IF_TYPE_AP);
+
+		// BUG: somewhat racey.. hostap should just set dev_addr itself
 		memcpy(new_dev->dev_addr, bss->bssid, ETH_ALEN);
+		dev_put(new_dev);
 		return 0;
 	case HOSTAP_IF_STA:
 		if (left < sizeof(struct hostapd_if_sta))
 			return -EPROTO;
 
-		res = ieee80211_if_add(dev, param->u.if_info.name, 0, &new_dev);
-		if (res)
-			return res;
-		ieee80211_if_set_type(new_dev, IEEE80211_IF_TYPE_STA);
-		return 0;
+		res = ieee80211_if_add(dev, param->u.if_info.name, NULL,
+				       IEEE80211_IF_TYPE_STA);
+		return res;
 	default:
 		return -EINVAL;
 	}
@@ -1093,36 +1103,34 @@ static int ieee80211_ioctl_update_if(struct net_device *dev,
 		struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 		struct net_device *wds_dev = NULL;
 		struct ieee80211_sub_if_data *sdata;
+		int ret;
 
 		if (left < sizeof(struct ieee80211_if_wds))
 			return -EPROTO;
 
+		read_lock(&local->sub_if_lock);
 		list_for_each_entry(sdata, &local->sub_if_list, list) {
 			if (strcmp(param->u.if_info.name,
 				   sdata->dev->name) == 0) {
 				wds_dev = sdata->dev;
+				dev_hold(wds_dev);
 				break;
 			}
 		}
+		read_unlock(&local->sub_if_lock);
 
 		if (!wds_dev || sdata->type != IEEE80211_IF_TYPE_WDS)
 			return -ENODEV;
 
-		return ieee80211_if_update_wds(wds_dev, wds->remote_addr);
+		ret = ieee80211_if_update_wds(wds_dev, wds->remote_addr);
+		dev_put(wds_dev);
+		return ret;
 	} else {
 		return -EOPNOTSUPP;
 	}
 }
 
 
-static int ieee80211_ioctl_flush_ifs(struct net_device *dev,
-				     struct prism2_hostapd_param *param)
-{
-	ieee80211_if_flush(dev);
-	return 0;
-}
-
-
 static int ieee80211_ioctl_scan_req(struct net_device *dev,
 				    struct prism2_hostapd_param *param,
 				    int param_len)
@@ -1512,9 +1520,6 @@ static int ieee80211_ioctl_priv_hostapd(struct net_device *dev,
 	case PRISM2_HOSTAPD_MLME:
 		ret = ieee80211_ioctl_mlme(dev, param);
 		break;
-	case PRISM2_HOSTAPD_FLUSH_IFS:
-		ret = ieee80211_ioctl_flush_ifs(dev, param);
-		break;
 	case PRISM2_HOSTAPD_SET_RADAR_PARAMS:
 		ret = ieee80211_ioctl_set_radar_params(dev, param);
 		break;
@@ -2239,6 +2244,7 @@ static int ieee80211_ioctl_clear_keys(struct net_device *dev)
 	struct sta_info *sta;
 
 	memset(addr, 0xff, ETH_ALEN);
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
 		for (i = 0; i < NUM_DEFAULT_KEYS; i++) {
 			keyconf = NULL;
@@ -2256,6 +2262,7 @@ static int ieee80211_ioctl_clear_keys(struct net_device *dev)
 		}
 		sdata->default_key = NULL;
 	}
+	read_unlock(&local->sub_if_lock);
 
 	spin_lock_bh(&local->sta_lock);
 	list_for_each_entry(sta, &local->sta_list, list) {
@@ -2388,6 +2395,7 @@ static int ieee80211_ioctl_default_wep_only(struct ieee80211_local *local,
 	struct ieee80211_sub_if_data *sdata;
 
 	local->default_wep_only = value;
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list)
 		for (i = 0; i < NUM_DEFAULT_KEYS; i++)
 			if (value)
@@ -2396,6 +2404,7 @@ static int ieee80211_ioctl_default_wep_only(struct ieee80211_local *local,
 			else
 				ieee80211_key_disable_hwaccel(local,
 							      sdata->keys[i]);
+	read_unlock(&local->sub_if_lock);
 
 	return 0;
 }
@@ -2406,7 +2415,7 @@ void ieee80211_update_default_wep_only(struct ieee80211_local *local)
 	int i = 0;
 	struct ieee80211_sub_if_data *sdata;
 
-	spin_lock_bh(&local->sub_if_lock);
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
 
 		if (sdata->dev == local->mdev)
@@ -2415,7 +2424,7 @@ void ieee80211_update_default_wep_only(struct ieee80211_local *local)
 		/* If there is an AP interface then depend on userspace to
 		   set default_wep_only correctly. */
 		if (sdata->type == IEEE80211_IF_TYPE_AP) {
-			spin_unlock_bh(&local->sub_if_lock);
+			read_unlock(&local->sub_if_lock);
 			return;
 		}
 
@@ -2427,7 +2436,7 @@ void ieee80211_update_default_wep_only(struct ieee80211_local *local)
 	else
 		ieee80211_ioctl_default_wep_only(local, 0);
 
-	spin_unlock_bh(&local->sub_if_lock);
+	read_unlock(&local->sub_if_lock);
 }
 
 
@@ -3287,4 +3296,5 @@ const struct iw_handler_def ieee80211_iw_handler_def =
 	.private	= (iw_handler *) ieee80211_private_handler,
 	.private_args	= (struct iw_priv_args *) ieee80211_ioctl_priv,
 	.get_wireless_stats = ieee80211_get_wireless_stats,
+	.no_locking	= 1,
 };
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index c41f3d3..a6ea77f 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -2728,7 +2728,7 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw)
 	memset(&wrqu, 0, sizeof(wrqu));
 	wireless_send_event(dev, SIOCGIWSCAN, &wrqu, NULL);
 
-	spin_lock_bh(&local->sub_if_lock);
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
 		if (sdata->type == IEEE80211_IF_TYPE_STA) {
 			if (sdata->u.sta.associated)
@@ -2737,7 +2737,7 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw)
 		}
 		netif_wake_queue(sdata->dev);
 	}
-	spin_unlock_bh(&local->sub_if_lock);
+	read_unlock(&local->sub_if_lock);
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	if (sdata->type == IEEE80211_IF_TYPE_IBSS) {
@@ -2873,14 +2873,14 @@ static int ieee80211_sta_start_scan(struct net_device *dev,
 
 	local->sta_scanning = 1;
 
-	spin_lock_bh(&local->sub_if_lock);
+	read_lock(&local->sub_if_lock);
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
 		netif_stop_queue(sdata->dev);
 		if (sdata->type == IEEE80211_IF_TYPE_STA &&
 		    sdata->u.sta.associated)
 			ieee80211_send_nullfunc(local, sdata, 1);
 	}
-	spin_unlock_bh(&local->sub_if_lock);
+	read_unlock(&local->sub_if_lock);
 
 	if (ssid) {
 		local->scan_ssid_len = ssid_len;
@@ -3124,7 +3124,6 @@ struct sta_info * ieee80211_ibss_add_sta(struct net_device *dev,
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct sta_info *sta;
 	struct ieee80211_sub_if_data *sdata = NULL;
-	struct net_device *sta_dev = NULL;
 
 	/* TODO: Could consider removing the least recently used entry and
 	 * allow new one to be added. */
@@ -3136,26 +3135,13 @@ struct sta_info * ieee80211_ibss_add_sta(struct net_device *dev,
 		return NULL;
 	}
 
-	spin_lock_bh(&local->sub_if_lock);
-	list_for_each_entry(sdata, &local->sub_if_list, list)
-		if (sdata->type == IEEE80211_IF_TYPE_IBSS &&
-		    memcmp(bssid, sdata->u.sta.bssid, ETH_ALEN) == 0) {
-			sta_dev = sdata->dev;
-			break;
-		}
-	spin_unlock_bh(&local->sub_if_lock);
-
-	if (!sta_dev)
-		return NULL;
-
 	printk(KERN_DEBUG "%s: Adding new IBSS station " MAC_FMT " (dev=%s)\n",
-	       dev->name, MAC_ARG(addr), sta_dev->name);
+	       local->mdev->name, MAC_ARG(addr), dev->name);
 
 	sta = sta_info_add(local, dev, addr, GFP_ATOMIC);
 	if (!sta)
 		return NULL;
 
-	sta->dev = sta_dev;
 	sta->supp_rates = sdata->u.sta.supp_rates_bits;
 
 	rate_control_rate_init(sta, local);

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]