[PATCH] wifi: iwlwifi: mld: don't dereference link_sta->sta unchecked

[PATCH] wifi: iwlwifi: mld: don't dereference link_sta->sta unchecked

[Edwin Huish]

mld->fw_id_to_link_sta[] can hold an entry whose ->sta is not set.  The
driver already documents this in iwl_mld_rx_agg_session_expired():

	/* sta should be valid unless the following happens:
	 * The firmware asserts which triggers a reconfig flow, but
	 * the reconfig fails before we set the pointer to sta into
	 * the fw_id_to_link_sta pointer table. mac80211 can't stop
	 * A-MPDU and hence the timer continues to run. Then, the
	 * timer expires and sta is NULL.
	 */

Three other users of the table only check the link_sta pointer itself
and then go straight for ->sta:

  iwl_mld_teardown_tdls_peers(), iwl_mld_tdls_sta_count(),
  iwl_mld_mac80211_flush()

	link_sta = wiphy_dereference(mld->wiphy, mld->fw_id_to_link_sta[i]);
	if (IS_ERR_OR_NULL(link_sta))
		continue;

	if (!link_sta->sta->tdls)	/* ->sta may be NULL */

iwl_mld_tdls_sta_count() was hit in the field on an Intel BE201
(8086:7740) running firmware bz-b0-fm-c0-c101 (101.6e695a70.0), in
exactly the situation described above: the firmware asserted twice
within a second (0x85 ADVANCED_SYSASSERT followed by 0x20100508) and
the restart failed with "FW error in SYNC CMD INIT_EXTENDED_CFG_CMD".
About a minute later wpa_supplicant re-associated and issued
NL80211_CMD_SET_STATION to update the power save parameters, which
ends up in iwl_mld_power_build_cmd() -> iwl_mld_tdls_sta_count():

  BUG: kernel NULL pointer dereference, address: 0000000000000018
  #PF: supervisor read access in kernel mode
  Oops: 0000 [#1] SMP NOPTI
  CPU: 7 PID: 1740 Comm: wpa_supplicant
  RIP: 0010:iwl_mld_tdls_sta_count+0x48/0xd0 [iwlmld]
  RAX: 000000000000000c  RCX: ffff8c51a95ba0a8  RDX: 0000000000000000
  Call Trace:
   <TASK>
   iwl_mld_power_build_cmd+0x11f/0x480 [iwlmld]
   iwl_mld_update_mac_power+0x66/0xe0 [iwlmld]
   iwl_mld_mac80211_vif_cfg_changed+0x58/0x120 [iwlmld]
   ieee80211_vif_cfg_change_notify+0x10d/0x230 [mac80211]
   ieee80211_recalc_ps_vif+0x41/0x60 [mac80211]
   ieee80211_change_station+0x1e0/0x280 [mac80211]
   nl80211_set_station+0x4e2/0x660 [cfg80211]
   genl_family_rcv_msg_doit+0x120/0x190
  note: wpa_supplicant[1740] exited with irqs disabled

The registers match the source: RAX is the loop index (12), RCX is mld,
and RDX - the result of loading link_sta->sta - is zero.  The faulting
instruction reads sta->tdls at offset 0x18:

    6d70: test   %rdx,%rdx          ; link_sta != NULL checked
    6d73: je     6d8b
    6d75: mov    (%rdx),%rdx        ; link_sta->sta, NOT checked
    6d78: movzbl 0x18(%rdx),%edx    ; sta->tdls  <- faulting insn

Since the oops happened with interrupts disabled, the task died in
kernel mode without releasing the cfg80211/rtnl locks it held, which
then blocked gnome-shell on the leaked lock and froze the whole
desktop.  The impact is therefore not limited to a dropped connection.

Handle the entries the same way agg.c already does, and skip them.

Tested on the affected machine: builds cleanly against 7.0.0-38-generic,
all three sites now test ->sta before loading sta->tdls, and WiFi still
associates and reconnects normally.

Signed-off-by: Edwin Huish <edwinhuish@gmail.com>
---
 drivers/net/wireless/intel/iwlwifi/mld/mac80211.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

--- a/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mld/mac80211.c
@@ -913,6 +913,11 @@ static void iwl_mld_teardown_tdls_peers(struct iwl_mld *mld)
 		if (IS_ERR_OR_NULL(link_sta))
 			continue;
 
+		if (!link_sta->sta) {
+			WARN_ON_ONCE(1);
+			continue;
+		}
+
 		if (!link_sta->sta->tdls)
 			continue;
 
@@ -1630,6 +1635,11 @@ int iwl_mld_tdls_sta_count(struct iwl_mld *mld)
 		if (IS_ERR_OR_NULL(link_sta))
 			continue;
 
+		if (!link_sta->sta) {
+			WARN_ON_ONCE(1);
+			continue;
+		}
+
 		if (!link_sta->sta->tdls)
 			continue;
 
@@ -1937,6 +1947,11 @@ static void iwl_mld_mac80211_flush(struct ieee80211_hw *hw,
 		if (IS_ERR_OR_NULL(link_sta))
 			continue;
 
+		if (!link_sta->sta) {
+			WARN_ON_ONCE(1);
+			continue;
+		}
+
 		/* Check that the sta belongs to the given vif */
 		if (vif && vif != iwl_mld_sta_from_mac80211(link_sta->sta)->vif)
 			continue;
-- 
2.43.0