[PATCH 1/2] mac80211: fix recursive locking in ieee80211_sta_expire

[PATCH 1/2] mac80211: fix recursive locking in ieee80211_sta_expire

[Jiri Slaby]

Hi,

it's against -mm, what's current mac80211 git tree? It seems that
-mm has fresher material, than wireless-2.6...

--

ieee80211_sta_expire grabs local->sta_lock and calls sta_info_unlink
which tries to grab the lock again. (make available) and use
__sta_info_unlink variant instead.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Michael Wu <flamingice@sourmilk.net>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Jiri Benc <jbenc@suse.cz>
---
 net/mac80211/ieee80211_sta.c |    2 +-
 net/mac80211/sta_info.c      |    2 +-
 net/mac80211/sta_info.h      |    2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index f9cf2f1..cfe6fcc 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -3081,7 +3081,7 @@ static void ieee80211_sta_expire(struct net_device *dev, unsigned long exp_time)
 		if (time_after(jiffies, sta->last_rx + exp_time)) {
 			printk(KERN_DEBUG "%s: expiring inactive STA %s\n",
 			       dev->name, print_mac(mac, sta->addr));
-			sta_info_unlink(&sta);
+			__sta_info_unlink(&sta);
 			if (sta)
 				list_add(&sta->list, &tmp_list);
 		}
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index f708367..ba02b43 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -409,7 +409,7 @@ static struct sta_info *__sta_info_unpin(struct sta_info *sta)
 	return ret;
 }
 
-static void __sta_info_unlink(struct sta_info **sta)
+void __sta_info_unlink(struct sta_info **sta)
 {
 	struct ieee80211_local *local = (*sta)->local;
 	struct ieee80211_sub_if_data *sdata = (*sta)->sdata;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 5e39a41..3a89b28 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -347,6 +347,8 @@ int sta_info_insert(struct sta_info *sta);
  * has already unlinked it.
  */
 void sta_info_unlink(struct sta_info **sta);
+/* no lock version */
+void __sta_info_unlink(struct sta_info **sta);
 
 void sta_info_destroy(struct sta_info *sta);
 void sta_info_set_tim_bit(struct sta_info *sta);
-- 
1.5.4.4

[PATCH 2/2] mac80211: fix defined but not used

[Jiri Slaby]

These two symbols are used only in ifdeffed function. Move them to that
section too.
net/mac80211/sta_info.c:387: warning: `__sta_info_pin' defined but not used
net/mac80211/sta_info.c:397: warning: `__sta_info_unpin' defined but not used

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Michael Wu <flamingice@sourmilk.net>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Jiri Benc <jbenc@suse.cz>
---
 net/mac80211/sta_info.c |   60 +++++++++++++++++++++++-----------------------
 1 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index ba02b43..e30426e 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -379,36 +379,6 @@ void sta_info_clear_tim_bit(struct sta_info *sta)
 	spin_unlock_irqrestore(&sta->local->sta_lock, flags);
 }
 
-/*
- * See comment in __sta_info_unlink,
- * caller must hold local->sta_lock.
- */
-static void __sta_info_pin(struct sta_info *sta)
-{
-	WARN_ON(sta->pin_status != STA_INFO_PIN_STAT_NORMAL);
-	sta->pin_status = STA_INFO_PIN_STAT_PINNED;
-}
-
-/*
- * See comment in __sta_info_unlink, returns sta if it
- * needs to be destroyed.
- */
-static struct sta_info *__sta_info_unpin(struct sta_info *sta)
-{
-	struct sta_info *ret = NULL;
-	unsigned long flags;
-
-	spin_lock_irqsave(&sta->local->sta_lock, flags);
-	WARN_ON(sta->pin_status != STA_INFO_PIN_STAT_DESTROY &&
-		sta->pin_status != STA_INFO_PIN_STAT_PINNED);
-	if (sta->pin_status == STA_INFO_PIN_STAT_DESTROY)
-		ret = sta;
-	sta->pin_status = STA_INFO_PIN_STAT_NORMAL;
-	spin_unlock_irqrestore(&sta->local->sta_lock, flags);
-
-	return ret;
-}
-
 void __sta_info_unlink(struct sta_info **sta)
 {
 	struct ieee80211_local *local = (*sta)->local;
@@ -565,6 +535,36 @@ static void sta_info_cleanup(unsigned long data)
 }
 
 #ifdef CONFIG_MAC80211_DEBUGFS
+/*
+ * See comment in __sta_info_unlink,
+ * caller must hold local->sta_lock.
+ */
+static void __sta_info_pin(struct sta_info *sta)
+{
+	WARN_ON(sta->pin_status != STA_INFO_PIN_STAT_NORMAL);
+	sta->pin_status = STA_INFO_PIN_STAT_PINNED;
+}
+
+/*
+ * See comment in __sta_info_unlink, returns sta if it
+ * needs to be destroyed.
+ */
+static struct sta_info *__sta_info_unpin(struct sta_info *sta)
+{
+	struct sta_info *ret = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sta->local->sta_lock, flags);
+	WARN_ON(sta->pin_status != STA_INFO_PIN_STAT_DESTROY &&
+		sta->pin_status != STA_INFO_PIN_STAT_PINNED);
+	if (sta->pin_status == STA_INFO_PIN_STAT_DESTROY)
+		ret = sta;
+	sta->pin_status = STA_INFO_PIN_STAT_NORMAL;
+	spin_unlock_irqrestore(&sta->local->sta_lock, flags);
+
+	return ret;
+}
+
 static void sta_info_debugfs_add_work(struct work_struct *work)
 {
 	struct ieee80211_local *local =
-- 
1.5.4.4

Re: [PATCH 1/2] mac80211: fix recursive locking in ieee80211_sta_expire

[drago01]

On Mon, Apr 7, 2008 at 9:53 PM, Jiri Slaby <jirislaby@gmail.com> wrote:
> Hi,
>
>  it's against -mm, what's current mac80211 git tree? It seems that
>  -mm has fresher material, than wireless-2.6...

The current tree is wireless-testing

Re: [PATCH 1/2] mac80211: fix recursive locking in ieee80211_sta_expire

[Johannes Berg]

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


> ieee80211_sta_expire grabs local->sta_lock and calls sta_info_unlink
> which tries to grab the lock again. (make available) and use
> __sta_info_unlink variant instead.

A patch equivalent to this should already be in.

johannes

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

Re: [PATCH 1/2] mac80211: fix recursive locking in ieee80211_sta_expire

[Jiri Slaby]

On 04/08/2008 04:29 PM, Johannes Berg wrote:
>> ieee80211_sta_expire grabs local->sta_lock and calls sta_info_unlink
>> which tries to grab the lock again. (make available) and use
>> __sta_info_unlink variant instead.
> 
> A patch equivalent to this should already be in.

Yeah, it is. I already replied to myself after getting to know about 
wireless-testing, but forgot to click "send". Thanks.

Re: [PATCH 1/2] mac80211: fix recursive locking in ieee80211_sta_expire

[John W. Linville]

On Mon, Apr 07, 2008 at 09:53:48PM +0200, Jiri Slaby wrote:
> Hi,
> 
> it's against -mm, what's current mac80211 git tree? It seems that
> -mm has fresher material, than wireless-2.6...

As it should.  The wireless-2.6 tree is for feeding the linux-2.6 tree
(by way of the net-2.6 tree).  Similarly, wireless-2.6.26 feeds -mm
by way of the net-2.6.26 tree.

Please use the wireless-testing tree for development, as it contains
both sets of patches (fixes for the current release and updates for the
future) as well as a few extras (like drivers still in development).
Plus it is somewhat more stable, being based off the most a recent
-rc release rather than just whatever someone last pulled.

Hth!

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH 1/2] mac80211: fix recursive locking in ieee80211_sta_expire

[Jiri Slaby]

On 04/08/2008 08:35 PM, John W. Linville wrote:
> On Mon, Apr 07, 2008 at 09:53:48PM +0200, Jiri Slaby wrote:
>> Hi,
>>
>> it's against -mm, what's current mac80211 git tree? It seems that
>> -mm has fresher material, than wireless-2.6...
> 
> As it should.  The wireless-2.6 tree is for feeding the linux-2.6 tree
> (by way of the net-2.6 tree).  Similarly, wireless-2.6.26 feeds -mm
> by way of the net-2.6.26 tree.

Wow, too many trees even with their branches to know the ropes.

> Please use the wireless-testing tree for development, as it contains
> both sets of patches (fixes for the current release and updates for the
> future) as well as a few extras (like drivers still in development).
> Plus it is somewhat more stable, being based off the most a recent
> -rc release rather than just whatever someone last pulled.

Ok, thanks.

Re: [PATCH 1/2] mac80211: fix recursive locking in ieee80211_sta_expire

[John W. Linville]

On Wed, Apr 09, 2008 at 12:06:03AM +0200, Jiri Slaby wrote:
> On 04/08/2008 08:35 PM, John W. Linville wrote:
>> On Mon, Apr 07, 2008 at 09:53:48PM +0200, Jiri Slaby wrote:
>>> Hi,
>>>
>>> it's against -mm, what's current mac80211 git tree? It seems that
>>> -mm has fresher material, than wireless-2.6...
>>
>> As it should.  The wireless-2.6 tree is for feeding the linux-2.6 tree
>> (by way of the net-2.6 tree).  Similarly, wireless-2.6.26 feeds -mm
>> by way of the net-2.6.26 tree.
>
> Wow, too many trees even with their branches to know the ropes.

Pick your poison -- previously everyone complained about the number
of branches in the tree.

FWIW, I've had far fewer complaints about tree management since
switching to the new practices.  Of course, most of that is probably
attributable to pleasing dwmw2 (more-or-less)...

John
-- 
John W. Linville
linville@tuxdriver.com