[PATCH] d80211: Fix inconsistent sta_lock usage

[Jan Kiszka <jan.kiszka@web.de>]

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

Hacking a bit on rt2x00 to make it work in master and ad-hoc mode, lockdep
popped up on some hostapd ioctls, pointing out remaining inconsistencies
related to sta_lock:

1. sta_lock holders must always be protected against softirq
2. bss_tim_set/clear must not be called with sta_lock held, rather an
   unprotected variant
3. ieee80211_ioctl_remove_sta is not already holding the lock when calling
   sta_info_free

As I was not sure if sta_info_remove_aid_ptr needs lock protection or
not, I played safe and moved it always under the lock. Please correct me
if this is overkill.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>

[Sorry, patch is against rt2x00 CVS. I'm lacking time and bandwidth to pull
the d80211 git repos and rebase.]

---
 ieee80211/ieee80211_i.h     |   24 ++++++++++++++++++------
 ieee80211/ieee80211_ioctl.c |    4 +++-
 ieee80211/sta_info.c        |    2 +-
 3 files changed, 22 insertions(+), 8 deletions(-)

Index: rt2x00/ieee80211/ieee80211_ioctl.c
===================================================================
--- rt2x00.orig/ieee80211/ieee80211_ioctl.c
+++ rt2x00/ieee80211/ieee80211_ioctl.c
@@ -286,7 +286,9 @@ static int ieee80211_ioctl_add_sta(struc
 	if (sta->dev != dev) {
 		/* Binding STA to a new interface, so remove all references to
 		 * the old BSS. */
+		spin_lock_bh(&local->sta_lock);
 		sta_info_remove_aid_ptr(sta);
+		spin_unlock_bh(&local->sta_lock);
 	}
 
         /* TODO
@@ -360,7 +362,7 @@ static int ieee80211_ioctl_remove_sta(st
 	sta = sta_info_get(local, param->sta_addr);
 	if (sta) {
 		sta_info_put(sta);
-		sta_info_free(sta, 1);
+		sta_info_free(sta, 0);
 	}
 
 	return sta ? 0 : -ENOENT;
Index: rt2x00/ieee80211/ieee80211_i.h
===================================================================
--- rt2x00.orig/ieee80211/ieee80211_i.h
+++ rt2x00/ieee80211/ieee80211_i.h
@@ -565,20 +565,32 @@ struct sta_attribute {
 	ssize_t (*store)(struct sta_info *, const char *buf, size_t count);
 };
 
+static inline void __bss_tim_set(struct ieee80211_local *local,
+				 struct ieee80211_if_ap *bss, int aid)
+{
+	bss->tim[(aid)/8] |= 1<<((aid) % 8);
+}
+
 static inline void bss_tim_set(struct ieee80211_local *local,
 			       struct ieee80211_if_ap *bss, int aid)
 {
-	spin_lock(&local->sta_lock);
-	bss->tim[(aid)/8] |= 1<<((aid) % 8);
-	spin_unlock(&local->sta_lock);
+	spin_lock_bh(&local->sta_lock);
+	__bss_tim_set(local, bss, aid);
+	spin_unlock_bh(&local->sta_lock);
+}
+
+static inline void __bss_tim_clear(struct ieee80211_local *local,
+				   struct ieee80211_if_ap *bss, int aid)
+{
+	bss->tim[(aid)/8] &= !(1<<((aid) % 8));
 }
 
 static inline void bss_tim_clear(struct ieee80211_local *local,
 				 struct ieee80211_if_ap *bss, int aid)
 {
-	spin_lock(&local->sta_lock);
-	bss->tim[(aid)/8] &= !(1<<((aid) % 8));
-	spin_unlock(&local->sta_lock);
+	spin_lock_bh(&local->sta_lock);
+	__bss_tim_clear(local, bss, aid);
+	spin_unlock_bh(&local->sta_lock);
 }
 
 /* ieee80211.c */
Index: rt2x00/ieee80211/sta_info.c
===================================================================
--- rt2x00.orig/ieee80211/sta_info.c
+++ rt2x00/ieee80211/sta_info.c
@@ -439,7 +439,7 @@ void sta_info_remove_aid_ptr(struct sta_
 		sdata->local->ops->set_tim(local_to_hw(sdata->local),
 					  sta->aid, 0);
 	if (sdata->bss)
-		bss_tim_clear(sdata->local, sdata->bss, sta->aid);
+		__bss_tim_clear(sdata->local, sdata->bss, sta->aid);
 }
 
 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]