Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   greearb@candelatech.com
To:     linux-wireless@vger.kernel.org
Cc:     Ben Greear <greearb@candelatech.com>
Subject: [PATCH] mac80211:  Remove vdevs from driver when firmware is unrecoverable.
Date:   Wed, 15 Jul 2020 09:45:31 -0700
Message-Id: <20200715164531.24883-1-greearb@candelatech.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

From: Ben Greear <greearb@candelatech.com>

While testing ax200 (backport version hacked into a 5.4 kernel),
I noticed a KASAN use-after-free.  I think this is because the
old logic would clear the SDATA_IN_DRIVER flag without actually
removing it from the driver.  Then a bit later, the driver would
try to access some sdata memory.

So instead, tell the driver to remove the vdevs.  The kernel still
splats a lot of warnings in this case, but I did not see a KASAN
error and the system recovered and continued running on the remaining
radio (the one w/out fatal firmware error).

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 net/mac80211/util.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index d8b411a13784..0f5869954df3 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2190,7 +2190,7 @@ static void ieee80211_flush_completed_scan(struct ieee80211_local *local,
 
 static void ieee80211_handle_reconfig_failure(struct ieee80211_local *local)
 {
-	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_sub_if_data *sdata, *sdata_tmp;
 	struct ieee80211_chanctx *ctx;
 
 	/*
@@ -2214,8 +2214,9 @@ static void ieee80211_handle_reconfig_failure(struct ieee80211_local *local)
 	 */
 	ieee80211_sched_scan_end(local);
 
-	list_for_each_entry(sdata, &local->interfaces, list)
-		sdata->flags &= ~IEEE80211_SDATA_IN_DRIVER;
+	list_for_each_entry_safe(sdata, sdata_tmp, &local->interfaces, list)
+		if (check_sdata_in_driver(sdata))
+			drv_remove_interface(local, sdata);
 
 	/* Mark channel contexts as not being in the driver any more to avoid
 	 * removing them from the driver during the shutdown process...
@@ -2381,6 +2382,10 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 			WARN(1, "Hardware became unavailable upon resume. This could be a software issue prior to suspend or a hardware issue.\n");
 		else
 			WARN(1, "Hardware became unavailable during restart.\n");
+		/* TODO: Requires driver reload and/or reboot to recover at this point.  Need
+		 * to notify user-space or set debugfs flag to WDT can be kicked in non-attended
+		 * devices such as APs... --Ben
+		 */
 		ieee80211_handle_reconfig_failure(local);
 		return res;
 	}
@@ -2425,12 +2430,6 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 	 * report failure.
 	 */
 	if (res) {
-		list_for_each_entry_continue_reverse(sdata, &local->interfaces,
-						     list)
-			if (sdata->vif.type != NL80211_IFTYPE_AP_VLAN &&
-			    sdata->vif.type != NL80211_IFTYPE_MONITOR &&
-			    ieee80211_sdata_running(sdata))
-				drv_remove_interface(local, sdata);
 		ieee80211_handle_reconfig_failure(local);
 		return res;
 	}
-- 
2.20.1