diff mbox series

mac80211: wind down station state earlier

Message ID 20201110105138.3f84f2055a30.I65a9d01564067122c47d0b5b6448bb4702a06b02@changeid
State New
Headers show
Series mac80211: wind down station state earlier | expand

Commit Message

Johannes Berg Nov. 10, 2020, 9:51 a.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

sta_info_free() may be used in a context that cannot sleep,
so do the wind-down earlier in __cleanup_single_sta(). We
should probably just unify that, by doing the free inside
sta_info_insert_finish() instead of the caller, but that'll
be a separate cleanup patch.

Reported-by: syzbot+6a7fe9faf0d1d61bc24a@syzkaller.appspotmail.com
Reported-by: syzbot+abed06851c5ffe010921@syzkaller.appspotmail.com
Reported-by: syzbot+b7aeb9318541a1c709f1@syzkaller.appspotmail.com
Reported-by: syzbot+d5a9416c6cafe53b5dd0@syzkaller.appspotmail.com
Fixes: dcd479e10a05 ("mac80211: always wind down STA state")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/sta_info.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

Comments

Johannes Berg Nov. 12, 2020, 8:26 a.m. UTC | #1 
On Tue, 2020-11-10 at 10:51 +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>

> 

> sta_info_free() may be used in a context that cannot sleep,

> so do the wind-down earlier in __cleanup_single_sta(). We

> should probably just unify that, by doing the free inside

> sta_info_insert_finish() instead of the caller, but that'll

> be a separate cleanup patch.

> 

> Reported-by: syzbot+6a7fe9faf0d1d61bc24a@syzkaller.appspotmail.com

> Reported-by: syzbot+abed06851c5ffe010921@syzkaller.appspotmail.com

> Reported-by: syzbot+b7aeb9318541a1c709f1@syzkaller.appspotmail.com

> Reported-by: syzbot+d5a9416c6cafe53b5dd0@syzkaller.appspotmail.com


That's probably true - however, it will get the original problem that
syzbot reported back ...

https://syzkaller.appspot.com/bug?extid=2e293dbd67de2836ba42

That's because the allocation is in pre_move_state().

So we can't do this - but need to do the other two patches I posted
directly.

johannes
diff mbox series

Patch

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 4fe284ff1ea3..377ec1aea9c6 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -145,6 +145,24 @@  static void __cleanup_single_sta(struct sta_info *sta)
 		ieee80211_purge_tx_queue(&local->hw, &tid_tx->pending);
 		kfree(tid_tx);
 	}
+
+	/*
+	 * If we had used sta_info_pre_move_state() then we might not
+	 * have gone through the state transitions down again, so do
+	 * it here now (and warn if it's inserted).
+	 *
+	 * This will clear state such as fast TX/RX that may have been
+	 * allocated during state transitions.
+	 */
+	while (sta->sta_state > IEEE80211_STA_NONE) {
+		int ret;
+
+		WARN_ON_ONCE(test_sta_flag(sta, WLAN_STA_INSERTED));
+
+		ret = sta_info_move_state(sta, sta->sta_state - 1);
+		if (WARN_ONCE(ret, "sta_info_move_state() returned %d\n", ret))
+			break;
+	}
 }
 
 static void cleanup_single_sta(struct sta_info *sta)
@@ -258,24 +276,6 @@  struct sta_info *sta_info_get_by_idx(struct ieee80211_sub_if_data *sdata,
  */
 void sta_info_free(struct ieee80211_local *local, struct sta_info *sta)
 {
-	/*
-	 * If we had used sta_info_pre_move_state() then we might not
-	 * have gone through the state transitions down again, so do
-	 * it here now (and warn if it's inserted).
-	 *
-	 * This will clear state such as fast TX/RX that may have been
-	 * allocated during state transitions.
-	 */
-	while (sta->sta_state > IEEE80211_STA_NONE) {
-		int ret;
-
-		WARN_ON_ONCE(test_sta_flag(sta, WLAN_STA_INSERTED));
-
-		ret = sta_info_move_state(sta, sta->sta_state - 1);
-		if (WARN_ONCE(ret, "sta_info_move_state() returned %d\n", ret))
-			break;
-	}
-
 	if (sta->rate_ctrl)
 		rate_control_free_sta(sta);