[PATCH] mac80211: wind down station state earlier

[PATCH] mac80211: wind down station state earlier

[Johannes Berg]

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(-)

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);
 
-- 
2.26.2

Re: [PATCH] mac80211: wind down station state earlier

[Johannes Berg]

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