[PATCH] wifi: mt76: mt7921: refactor regd update to fix recursive mutex deadlock

[PATCH] wifi: mt76: mt7921: refactor regd update to fix recursive mutex deadlock

[JB Tsai]

From: Charlie-cy Wu <charlie-cy.wu@mediatek.corp-partner.google.com>

Split mt7921_mcu_regd_update() into two functions to prevent recursive
mutex acquisition. Introduce __mt7921_mcu_regd_update() as the internal
implementation that assumes the mutex is already held by the caller,
while mt7921_mcu_regd_update() remains as the external interface that
handles mutex acquisition and release.

This fixes a deadlock issue when mt7921_regd_set_6ghz_power_type() is
called with the device mutex already held. Without this change, calling
mt7921_mcu_regd_update() would attempt to acquire the same mutex again,
causing a recursive lock deadlock.

The __mt7921_mcu_regd_update() function can be safely called when the
caller has already acquired the device mutex, avoiding the deadlock
while maintaining proper synchronization for regulatory domain updates.

Fixes: dc2608cf5224 ("wifi: mt76: mt7921: refactor regulatory notifier flow")
Signed-off-by: Charlie-cy Wu <Charlie-cy.Wu@mediatek.com>
---
 .../net/wireless/mediatek/mt76/mt7921/main.c  |  2 +-
 .../net/wireless/mediatek/mt76/mt7921/regd.c  | 30 ++++++++++++-------
 .../net/wireless/mediatek/mt76/mt7921/regd.h  |  2 ++
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
index af5d16055396..b3f29ebf4015 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
@@ -808,7 +808,7 @@ mt7921_regd_set_6ghz_power_type(struct ieee80211_vif *vif, bool is_add)
 
 out:
 	if (vif->bss_conf.chanreq.oper.chan->band == NL80211_BAND_6GHZ)
-		mt7921_mcu_regd_update(dev, dev->mt76.alpha2, dev->country_ie_env);
+		__mt7921_mcu_regd_update(dev, dev->mt76.alpha2, dev->country_ie_env);
 }
 
 int mt7921_mac_sta_add(struct mt76_dev *mdev, struct ieee80211_vif *vif,
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/regd.c b/drivers/net/wireless/mediatek/mt76/mt7921/regd.c
index f122e418d825..f923af1440d7 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/regd.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/regd.c
@@ -71,36 +71,44 @@ mt7921_regd_channel_update(struct wiphy *wiphy, struct mt792x_dev *dev)
 	}
 }
 
-int mt7921_mcu_regd_update(struct mt792x_dev *dev, u8 *alpha2,
-			   enum environment_cap country_ie_env)
+/* Internal version that assumes mutex is already held by caller */
+int __mt7921_mcu_regd_update(struct mt792x_dev *dev, u8 *alpha2,
+			     enum environment_cap country_ie_env)
 {
 	struct mt76_dev *mdev = &dev->mt76;
 	struct ieee80211_hw *hw = mdev->hw;
 	struct wiphy *wiphy = hw->wiphy;
 	int ret = 0;
 
-	dev->regd_in_progress = true;
-
-	mt792x_mutex_acquire(dev);
 	if (!dev->regd_change)
-		goto err;
+		return 0;
 
 	ret = mt7921_mcu_set_clc(dev, alpha2, country_ie_env);
 	if (ret < 0)
-		goto err;
+		return ret;
 
 	mt7921_regd_channel_update(wiphy, dev);
 
 	ret = mt76_connac_mcu_set_channel_domain(hw->priv);
 	if (ret < 0)
-		goto err;
+		return ret;
 
 	ret = mt7921_set_tx_sar_pwr(hw, NULL);
-	if (ret < 0)
-		goto err;
 
-err:
+	return ret;
+}
+
+int mt7921_mcu_regd_update(struct mt792x_dev *dev, u8 *alpha2,
+			   enum environment_cap country_ie_env)
+{
+	int ret = 0;
+
+	dev->regd_in_progress = true;
+
+	mt792x_mutex_acquire(dev);
+	ret = __mt7921_mcu_regd_update(dev, alpha2, country_ie_env);
 	mt792x_mutex_release(dev);
+
 	dev->regd_change = false;
 	dev->regd_in_progress = false;
 	wake_up(&dev->wait);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/regd.h b/drivers/net/wireless/mediatek/mt76/mt7921/regd.h
index 571f31629e9e..5b24d0902c36 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/regd.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/regd.h
@@ -10,6 +10,8 @@ struct regulatory_request;
 
 int mt7921_mcu_regd_update(struct mt792x_dev *dev, u8 *alpha2,
 			   enum environment_cap country_ie_env);
+int __mt7921_mcu_regd_update(struct mt792x_dev *dev, u8 *alpha2,
+			     enum environment_cap country_ie_env);
 void mt7921_regd_notifier(struct wiphy *wiphy,
 			  struct regulatory_request *request);
 bool mt7921_regd_clc_supported(struct mt792x_dev *dev);
-- 
2.45.2

Re: [PATCH] wifi: mt76: mt7921: refactor regd update to fix recursive mutex deadlock

[Devin Wittmayer]

This one is still missing from mainline, and the bug it fixes is in 7.2.
Could it go into a fixes pull before the release rather than waiting for
7.3?

Current master still has mt7921_mcu_regd_update() taking dev->mutex
unconditionally, and mt7921_regd_set_6ghz_power_type() still calling it
from mt7921_mac_sta_add(), which runs with that same mutex already held
by mt76_sta_state(). Any mt7921 station that associates to a 6 GHz AP
self-deadlocks. It is not only a warning: wpa_supplicant blocks while
holding the wiphy mutex, so NetworkManager, the cfg80211 workqueue and
new ssh sessions pile up behind it and the machine has to be reset. The
call site arrived in e88098133ed4, which is in v7.2-rc1 and not in v7.1,
so nothing released is affected and no stable backport is needed, but
7.2 would ship it.

I reproduced it on 7.2-rc5 with lockdep on an MT7922, then applied this
patch and repeated the same association. Before, the recursive acquire
fires and the box is gone. After, it associates on 5975 MHz in four
seconds with no splat and nothing left blocked. Nothing else changed
between the two runs.

Tested-by: Devin Wittmayer <lucid_duck@justthetip.ca>

One small thing for whenever it moves: the Fixes tag cites dc2608cf5224,
which is not a mainline commit. The mainline id for "wifi: mt76: mt7921:
refactor regulatory notifier flow" is e88098133ed4.

Re: [PATCH] wifi: mt76: mt7921: refactor regd update to fix recursive mutex deadlock

[Mikhail Gavrilov]

I hit this deadlock on an MT7922 (mt7921e) and arrived at the same split
independently before finding this commit in linux-next.

Without it, associating with a 6 GHz AP self-deadlocks: mt76_sta_state()
holds &dev->mt76.mutex and mt7921_regd_set_6ghz_power_type() takes it
again via mt7921_mcu_regd_update(), on both the sta_add and the
sta_remove path. wpa_supplicant then blocks while holding wiphy.mtx,
NetworkManager blocks on wiphy.mtx while holding rtnl, and the machine
is left with no working network stack and needs sysrq to reboot. Because
NetworkManager retries the saved profile on every boot, an affected
kernel stops reaching a usable state at all once a 6 GHz profile exists.

With this commit applied to mainline at 3eb40771c00a (post-v7.2-rc7), on
a lockdep and UBSAN build: association with a 6 GHz AP completes
(channel 37, 6135 MHz, 160 MHz), switching back and forth between a
5 GHz and a 6 GHz BSS exercises both call sites, "iw reg set NL" reaches
the regulatory notifier and visibly takes effect - the link moves off
6 GHz - and "iw reg set RU" restores it, and an 88 s deep suspend/resume
cycle reconnects to the 6 GHz BSS. dmesg is clean throughout.

One request while this is fresh: the regression this fixes was
introduced in v7.2-rc1 by e88098133ed4, so it ought to be fixed within
7.2 rather than carried into 7.3. This commit is only in wireless-next
and has no Cc: stable. Could it go in through the fixes tree before the
7.2 release, or failing that into 7.2.y as soon as it opens?

Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>