Linux wireless drivers development
 help / color / mirror / Atom feed
From: Cristian Papa <pcristian292@gmail.com>
To: nbd@nbd.name
Cc: lorenzo@kernel.org, ryder.lee@mediatek.com,
	shayne.chen@mediatek.com, sean.wang@mediatek.com,
	linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: [PATCH] wifi: mt76: mt76x02: keep stations registered during watchdog recovery
Date: Wed, 14 Oct 2026 11:42:06 -0300	[thread overview]
Message-ID: <20261014144206.7401-1-pcristian292@gmail.com> (raw)

The full watchdog recovery in mt76x02_reset_state() unpublishes every
station WCID, removes it with __mt76_sta_remove() and clears its driver
data, expecting ieee80211_restart_hw() to add it back. mac80211 still
owns these stations, though, and dev->mutex is released before the
restart is requested. A station removed in that interval reaches
mt76_wcid_cleanup() with zeroed list heads:

  [   66.096357] CPU 1 Unable to handle kernel paging request at virtual address 00000000, epc == 849063c4, ra == 849063b0
  [   66.107085] Oops[#1]:
  [   66.109425] CPU: 1 UID: 0 PID: 4280 Comm: iw Tainted: G           O        6.18.44 #0 NONE
  [   66.480557] Code: 1282000d  02a02025  8e42029c <8c440000> 1684007f  8e430298  8c640004  1684007c  00000000
  [   66.496876] Kernel panic - not syncing: Fatal exception in interrupt

The faulting load is the CONFIG_LIST_HARDENED check of
list_del_init(&wcid->poll_list) in mt76_wcid_cleanup(), called from
__mt76_sta_remove() <- mt76_sta_remove() <- mt76_sta_state().

A station added in that interval is replayed by mac80211 afterwards as
well: mt76x02_sta_add() clears it again and allocates a second WCID, so
the station ends up published under two indexes, one of which leaks, and
is counted twice in phy->num_sta.

Keep the stations registered instead. During recovery, stop their RX
reorder sessions, drop and clear their hardware WCID entries and flush
them with mt76_wcid_cleanup(), which leaves the lists, queues and packet
IDR initialized. Also clear the cached TX status aggregation, so the
first status after recovery is not merged with one from before it.

When mac80211 replays a station whose WCID is still published, only
restore its hardware entry, honouring the preserved power-save state.
Count a station in mt76_sta_add() only on its initial registration, and
check for an existing publication before the driver callback, since some
drivers publish a newly allocated WCID from there.

Fixes: 004960423fe1 ("mt76: mt76x2: implement full device restart on watchdog reset")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Cristian Papa <pcristian292@gmail.com>
---
Based on mt76 master 467ae2e3f190.

Testing: TP-Link Archer XR500v, MT7612E (mt76x2e), Linux 6.18.44 with
backports 7.2. A test-only module change forces a full watchdog recovery
and keeps dev->mutex for 3 s after mt76x02_reset_state(), while
`iw dev <ap> station del <sta>` removes an associated station inside
that window. The window is widened artificially; without the test change
it is much shorter, and this crash needs a recovery and a station removal
to coincide.

- mt76 master, unpatched: the router reboots with the oops above.
- mt76 master with this patch: 3/3 runs without a crash; the removed
  station reconnects and no warnings are logged.
- The same change on OpenWrt's mt76 (be5ce7910521): 4/4 crashes before,
  5/5 clean after.
- A forced recovery without a station removal does not crash, before or
  after the patch.

The orderings of recovery, removal, addition and replay were also
checked exhaustively in a host model (61 orderings): 28 fail without the
patch (crash, double publication, use-after-free, index leak, num_sta),
none with it.

AI assistance: LLM agents were used for the code analysis, the host
model and drafting. The tests ran on my hardware and I reviewed the
change.

 mac80211.c     |  6 +++++-
 mt76x02_mmio.c | 29 ++++++++++++++++-------------
 mt76x02_util.c | 14 +++++++++++++-
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
index 41d3636..d3ea506 100644
--- a/drivers/net/wireless/mediatek/mt76/mac80211.c
+++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
@@ -1610,11 +1610,14 @@ mt76_sta_add(struct mt76_phy *phy, struct ieee80211_vif *vif,
 	struct mt76_wcid *wcid = (struct mt76_wcid *)sta->drv_priv;
 	struct mt76_dev *dev = phy->dev;
 	struct mt76_wcid *published;
+	bool already_present;
 	int ret;
 	int i;
 
 	mutex_lock(&dev->mutex);
 
+	/* Before the driver callback: some drivers publish a new WCID there. */
+	already_present = rcu_access_pointer(dev->wcid[wcid->idx]) == wcid;
 	ret = dev->drv->sta_add(dev, vif, sta);
 	if (ret)
 		goto out;
@@ -1640,7 +1643,8 @@ mt76_sta_add(struct mt76_phy *phy, struct ieee80211_vif *vif,
 		wcid->phy_idx = phy->band_idx;
 	}
 
-	phy->num_sta++;
+	if (!already_present)
+		phy->num_sta++;
 
 out:
 	mutex_unlock(&dev->mutex);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
index dc7c03d..7392aae 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
@@ -403,27 +403,30 @@ static void mt76x02_reset_state(struct mt76x02_dev *dev)
 	rcu_read_unlock();
 
 	for (i = 0; i < MT76x02_N_WCIDS; i++) {
-		struct ieee80211_sta *sta;
-		struct ieee80211_vif *vif;
 		struct mt76x02_sta *msta;
 		struct mt76_wcid *wcid;
-		void *priv;
+		int tid;
 
 		wcid = rcu_dereference_protected(dev->mt76.wcid[i],
 					lockdep_is_held(&dev->mt76.mutex));
 		if (!wcid)
 			continue;
 
-		rcu_assign_pointer(dev->mt76.wcid[i], NULL);
-
-		priv = msta = container_of(wcid, struct mt76x02_sta, wcid);
-		sta = container_of(priv, struct ieee80211_sta, drv_priv);
-
-		priv = msta->vif;
-		vif = container_of(priv, struct ieee80211_vif, drv_priv);
-
-		__mt76_sta_remove(&dev->mphy, vif, sta);
-		memset(msta, 0, sizeof(*msta));
+		/* mac80211 still owns the station and may remove it before
+		 * ieee80211_restart_hw() replays it. Keep the WCID published
+		 * and its software state initialized; only flush the station
+		 * and clear its hardware entry.
+		 */
+		for (tid = 0; tid < ARRAY_SIZE(wcid->aggr); tid++)
+			mt76_rx_aggr_stop(&dev->mt76, wcid, tid);
+
+		mt76x02_mac_wcid_set_drop(dev, wcid->idx, true);
+		mt76x02_mac_wcid_setup(dev, wcid->idx, 0, NULL);
+		mt76_wcid_cleanup(&dev->mt76, wcid);
+
+		msta = container_of(wcid, struct mt76x02_sta, wcid);
+		memset(&msta->status, 0, sizeof(msta->status));
+		msta->n_frames = 0;
 	}
 
 	dev->mt76.vif_mask = 0;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index e5d9d1b..21bfa20 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -241,7 +241,19 @@ int mt76x02_sta_add(struct mt76_dev *mdev, struct ieee80211_vif *vif,
 	struct mt76x02_dev *dev = container_of(mdev, struct mt76x02_dev, mt76);
 	struct mt76x02_sta *msta = (struct mt76x02_sta *)sta->drv_priv;
 	struct mt76x02_vif *mvif = (struct mt76x02_vif *)vif->drv_priv;
-	int idx = 0;
+	int idx = msta->wcid.idx;
+
+	/* Recovery replays the station while it is still registered: keep
+	 * its WCID and software state, and only restore the hardware entry.
+	 */
+	if (mt76_is_mmio(mdev) &&
+	    rcu_access_pointer(mdev->wcid[idx]) == &msta->wcid) {
+		mt76x02_mac_wcid_setup(dev, idx, mvif->idx, sta->addr);
+		mt76x02_mac_wcid_set_drop(dev, idx,
+					  test_bit(MT_WCID_FLAG_PS,
+						   &msta->wcid.flags));
+		return 0;
+	}
 
 	memset(msta, 0, sizeof(*msta));
 
-- 
2.47.3


                 reply	other threads:[~2026-10-14 14:42 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261014144206.7401-1-pcristian292@gmail.com \
    --to=pcristian292@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=nbd@nbd.name \
    --cc=ryder.lee@mediatek.com \
    --cc=sean.wang@mediatek.com \
    --cc=shayne.chen@mediatek.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox