public inbox for linux-mediatek@lists.infradead.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Sean Wang <sean.wang@mediatek.com>, Felix Fietkau <nbd@nbd.name>,
	Sasha Levin <sashal@kernel.org>,
	lorenzo@kernel.org, ryder.lee@mediatek.com,
	matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com,
	linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: [PATCH AUTOSEL 7.0-6.12] wifi: mt76: mt7925: resolve link after acquiring mt76 mutex
Date: Mon, 20 Apr 2026 09:19:40 -0400	[thread overview]
Message-ID: <20260420132314.1023554-186-sashal@kernel.org> (raw)
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>

From: Sean Wang <sean.wang@mediatek.com>

[ Upstream commit beec58f36983f826fe90287a90edff46b32e8a89 ]

mt792x_sta_to_link() uses rcu_dereference_protected() and therefore
expects mt76.mutex to be held. Move the lookup after
mt792x_mutex_acquire() to make the locking explicit and correct.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Link: https://patch.msgid.link/20260306232238.2039675-12-sean.wang@kernel.org
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed for a thorough analysis.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem**: `wifi: mt76: mt7925` (MediaTek MT7925 WiFi driver in
  the mt76 subsystem)
- **Action verb**: "resolve" — implies correcting something that was
  wrong
- **Summary**: Move the `mt792x_sta_to_link()` lookup after acquiring
  `mt76.mutex`, because the function uses `rcu_dereference_protected()`
  which requires that lock.

Record: [wifi: mt76: mt7925] [resolve/correct] [Fix locking order:
acquire mutex before RCU-protected dereference]

### Step 1.2: Tags
- **Signed-off-by**: Sean Wang <sean.wang@mediatek.com> — the author, a
  prominent mt76/mt7925 contributor
- **Link**: https://patch.msgid.link/20260306232238.2039675-12-
  sean.wang@kernel.org — patch 12 (of 20 messages, patch 11/19)
- **Signed-off-by**: Felix Fietkau <nbd@nbd.name> — the mt76 subsystem
  maintainer who applied the patch
- No Fixes: tag, no Reported-by:, no Cc: stable — expected for candidate
  review

Record: Author is Sean Wang (regular mt76 contributor), applied by the
mt76 maintainer Felix Fietkau. No explicit Fixes: or stable tags.

### Step 1.3: Commit Body Analysis
The body says: "`mt792x_sta_to_link()` uses
`rcu_dereference_protected()` and therefore expects `mt76.mutex` to be
held." This directly describes a locking contract violation. The fix:
"Move the lookup after `mt792x_mutex_acquire()`."

Record: Bug: calling `rcu_dereference_protected()` without holding the
required lock. Symptom: lockdep warning if `CONFIG_PROVE_LOCKING` is
enabled; potential race condition for MLO vifs where the RCU pointer
could be concurrently modified.

### Step 1.4: Hidden Bug Fix Detection
This IS a bug fix despite not using the word "fix" — the commit corrects
a lock ordering violation. The `rcu_dereference_protected()` API
explicitly expects the lock to be held, and calling it without it is
incorrect.

Record: Yes, this is a real bug fix — locking correctness violation.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
- **File**: `drivers/net/wireless/mediatek/mt76/mt7925/main.c`
- **Function**: `mt7925_mac_link_sta_assoc()`
- **Change**: 2 lines moved (net 0 lines added/removed — pure reorder)
- **Scope**: Single function, single file, surgical

Record: 1 file changed, ~4 lines reordered within one function. Scope:
trivially small.

### Step 2.2: Code Flow Change
**Before**: `msta` and `mlink` were resolved via `mt792x_sta_to_link()`
before `mt792x_mutex_acquire(dev)` was called.

**After**: `mt792x_mutex_acquire(dev)` is called first, then `msta` and
`mlink` are resolved.

The `msta` assignment (`(struct mt792x_sta *)link_sta->sta->drv_priv`)
does not need the lock, but moving it below the mutex acquire is
harmless and keeps the code cleaner.

### Step 2.3: Bug Mechanism
This is a **synchronization / locking correctness fix**. The function
`mt792x_sta_to_link()` uses:

```293:294:drivers/net/wireless/mediatek/mt76/mt792x.h
return rcu_dereference_protected(msta->link[link_id],
        lockdep_is_held(&msta->vif->phy->dev->mt76.mutex));
```

The `rcu_dereference_protected()` call asserts that `mt76.mutex` must be
held. Calling it without the lock is:
1. A lockdep assertion violation (runtime warning with
   `CONFIG_PROVE_LOCKING`)
2. A potential race: without the mutex, the `msta->link[link_id]` RCU
   pointer could be concurrently modified (e.g., during link teardown),
   leading to use-after-free.

Note: For non-MLD (non-WiFi-7-MLO) vifs, the function returns
`&msta->deflink` early without touching RCU, so the actual RCU race only
applies to MLO connections.

Record: Synchronization/locking fix. `rcu_dereference_protected()`
called without required mutex. Race window for concurrent link
modification on MLO vifs.

### Step 2.4: Fix Quality
- **Obviously correct**: Yes — just reorders two existing operations.
- **Minimal**: Yes — net zero lines changed.
- **Regression risk**: Essentially zero. The mutex is acquired slightly
  earlier but still released at the same point. No new code is added.

Record: Fix is trivially correct with no regression risk.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
From the git blame, the buggy code was introduced by commit
`89397bccc882a4` ("wifi: mt76: mt7925: add mt7925_mac_link_sta_assoc to
associate per-link STA") by Sean Wang, dated 2024-06-12. This commit
first appeared in `v6.11-rc1`.

Record: Buggy code introduced in 89397bccc882a4, first in v6.11-rc1.
Present in stable trees v6.11+.

### Step 3.2: Fixes Tag
No explicit Fixes: tag, but the bug was clearly introduced by
`89397bccc882a4`.

Record: The original commit created the function
`mt7925_mac_link_sta_assoc` with the lock ordering issue from day one.

### Step 3.3: File History
The file has had ~46 commits since v6.11. Several are related locking
fixes (e.g., `9f15701370ec1` "fix locking in
mt7925_change_vif_links()").

Record: Active file with other locking fixes. This is standalone.

### Step 3.4: Author
Sean Wang is a prolific contributor to mt76/mt7925 and the author of the
MLO link support. He's also the author of the original buggy commit, so
this is the author fixing their own oversight.

Record: Author is the subsystem developer who introduced the bug.

### Step 3.5: Dependencies
This is patch 11/19 in a series, but the fix itself is **self-
contained**: it only reorders existing lines within one function. It has
no dependency on any other patch in the series.

Record: Self-contained, no dependencies.

---

## PHASE 4: MAILING LIST RESEARCH

### Step 4.1: Original Discussion
The patch was submitted as `[PATCH 11/19]` in a series titled "wifi:
mt76: mt7925: fix up MLO link lifetime and error handling". The series
includes a mix of refactoring and fixes. This specific patch is a pure
correctness fix.

Record: Part of a 19-patch series for MLO link lifetime improvements.
This patch is standalone.

### Step 4.2: Reviewers
The series was sent to `nbd@nbd.name` (Felix Fietkau, mt76 maintainer)
and `lorenzo.bianconi@redhat.com`. Applied by Felix Fietkau.

Record: Applied by subsystem maintainer.

### Step 4.3-4.5: Bug Reports / Stable Discussion
No specific bug report or syzbot link. No specific stable discussion
found. The kernel test robot reported build issues on patch 2/19 only,
not on this patch.

Record: No external bug reports. The issue is self-evident from code
inspection.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key Functions
- `mt7925_mac_link_sta_assoc()` — the function being fixed
- `mt792x_sta_to_link()` — the inline function that requires the mutex

### Step 5.2: Callers
`mt7925_mac_link_sta_assoc()` is called from `mt7925_mac_sta_event()`
(line 1078), which is exported via `EXPORT_SYMBOL_GPL` and called during
station association events via the mac80211 callback path. This is a
common WiFi operational path.

### Step 5.4: Reachability
The code path is: mac80211 sta_event callback ->
`mt7925_mac_sta_event()` -> `mt7925_mac_link_sta_assoc()`. This is
triggered during WiFi association, which is a very common operation.

Record: The buggy code is on a common WiFi association path, reachable
during normal operation.

---

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Presence in Stable Trees
- Buggy commit `89397bccc882a4` is NOT in v6.10 (verified)
- It IS in v6.11+ (verified: v6.11-rc1)
- Relevant stable trees: 6.11.y, 6.12.y, and any later LTS

### Step 6.2: Backport Complications
The patch is a trivial reorder of existing lines. It should apply
cleanly to any tree that has the buggy commit.

Record: Clean apply expected for all trees with the buggy code (v6.11+).

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem Criticality
- Subsystem: `drivers/net/wireless/mediatek/mt76/mt7925/` — WiFi driver
- Criticality: IMPORTANT — MediaTek MT7925 is a WiFi 7 chip used in many
  modern laptops
- The fix is specifically for the MLO (Multi-Link Operation) code path

Record: IMPORTANT subsystem — popular WiFi 7 chip. Bug affects MLO
connections.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who Is Affected
Users of MediaTek MT7925 WiFi 7 hardware using MLO (Multi-Link
Operation). For non-MLO connections, `mt792x_sta_to_link()` takes the
early `deflink` return path and doesn't touch RCU.

### Step 8.2: Trigger Conditions
- Triggered during WiFi association with MLO
- Common operation for WiFi 7 users
- With `CONFIG_PROVE_LOCKING`: always triggers a warning
- Without: race window exists but may be hard to hit

### Step 8.3: Failure Mode Severity
- **With lockdep**: WARNING (lock assertion failure) — MEDIUM
- **Without lockdep**: Potential use-after-free if link is concurrently
  modified — HIGH (crash/corruption risk, though race window is small)

### Step 8.4: Risk-Benefit Ratio
- **Benefit**: Fixes locking correctness and prevents lockdep warnings +
  potential UAF for MLO users. LOW-MEDIUM benefit.
- **Risk**: Essentially ZERO risk — the fix is a trivial reorder of two
  existing operations.
- **Ratio**: Very favorable — zero risk for a real correctness fix.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Summary

**FOR backporting:**
- Fixes a real locking correctness violation
  (`rcu_dereference_protected()` without lock)
- The fix is trivially correct — just reorders 2 lines
- Zero regression risk
- Self-contained, no dependencies on other patches
- Applied by subsystem maintainer
- Bug exists in stable trees v6.11+
- On a common WiFi association code path

**AGAINST backporting:**
- Part of a 19-patch series (though this patch is standalone)
- Primarily affects WiFi 7 MLO users (subset of mt7925 users)
- Without lockdep, the actual race is theoretically possible but hard to
  trigger
- No user reports or syzbot reports of the issue

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** — trivial reorder, applied by
   maintainer
2. Fixes a real bug? **YES** — locking correctness violation
3. Important issue? **MEDIUM** — lockdep warning + potential UAF race on
   MLO
4. Small and contained? **YES** — ~4 lines reordered in one function
5. No new features or APIs? **YES** — pure fix
6. Can apply to stable trees? **YES** — should apply cleanly to v6.11+

### Step 9.3: Exception Categories
Not an exception category — this is a standard bug fix.

### Step 9.4: Decision
The fix is a trivially correct locking fix with zero regression risk. It
fixes a real `rcu_dereference_protected()` locking violation that
triggers lockdep warnings and has a theoretical UAF race. While the
impact is limited to WiFi 7 MLO users, the fix is so small and safe that
the risk-benefit ratio strongly favors backporting.

---

## Verification

- [Phase 1] Parsed tags: Author is Sean Wang (MediaTek), applied by
  Felix Fietkau (mt76 maintainer)
- [Phase 2] Diff analysis: ~4 lines reordered in
  `mt7925_mac_link_sta_assoc()` — moves mutex_acquire before
  `mt792x_sta_to_link()`
- [Phase 2] Verified `mt792x_sta_to_link()` uses
  `rcu_dereference_protected()` with `lockdep_is_held(&mt76.mutex)` at
  lines 293-294 of mt792x.h
- [Phase 2] Verified `mt792x_mutex_acquire()` expands to
  `mutex_lock(&dev->mutex)` via `mt76_connac_mutex_acquire()` at line
  401 of mt76_connac.h
- [Phase 3] git blame: buggy code introduced by 89397bccc882a4 (Sean
  Wang, 2024-06-12)
- [Phase 3] git describe: buggy commit first appears in v6.11-rc1
- [Phase 3] git merge-base: confirmed NOT in v6.10, YES in v6.11 and
  v6.12
- [Phase 4] b4 mbox: found series cover "fix up MLO link lifetime and
  error handling" (19 patches)
- [Phase 4] This is patch 11/19, but is self-contained
- [Phase 5] `mt7925_mac_link_sta_assoc()` called from
  `mt7925_mac_sta_event()` (line 1078), triggered during WiFi
  association
- [Phase 6] Code exists in stable trees v6.11+
- [Phase 8] Failure mode: lockdep WARNING (MEDIUM) or potential UAF race
  on MLO (HIGH but hard to trigger)
- UNVERIFIED: Could not access lore.kernel.org directly due to anti-bot
  protection to check for reviewer comments on patch 11/19 specifically

**YES**

 drivers/net/wireless/mediatek/mt76/mt7925/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/main.c b/drivers/net/wireless/mediatek/mt76/mt7925/main.c
index 38474cd2c38d6..caafac14d97ba 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7925/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7925/main.c
@@ -1030,11 +1030,11 @@ static void mt7925_mac_link_sta_assoc(struct mt76_dev *mdev,
 	struct mt792x_link_sta *mlink;
 	struct mt792x_sta *msta;
 
+	mt792x_mutex_acquire(dev);
+
 	msta = (struct mt792x_sta *)link_sta->sta->drv_priv;
 	mlink = mt792x_sta_to_link(msta, link_sta->link_id);
 
-	mt792x_mutex_acquire(dev);
-
 	if (ieee80211_vif_is_mld(vif)) {
 		link_conf = mt792x_vif_to_bss_conf(vif, msta->deflink_id);
 	} else {
-- 
2.53.0



  parent reply	other threads:[~2026-04-20 13:29 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260420132314.1023554-1-sashal@kernel.org>
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-6.19] wifi: mt76: avoid to set ACK for MCU command if wait_resp is not set Sasha Levin
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-6.18] phy: phy-mtk-tphy: Update names and format of kernel-doc comments Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.12] Bluetooth: btmtk: add MT7902 MCU support Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: flush pending TX before channel switch Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.6] wifi: mt76: fix list corruption in mt76_wcid_cleanup Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.12] wifi: mt76: add missing lock protection in mt76_sta_state for sta_event callback Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.1] Bluetooth: btmtk: improve mt79xx firmware setup retry flow Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: Disable Rx hdr_trans in monitor mode Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.12] wifi: mt76: mt7925: Skip scan process during suspend Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-5.10] wifi: mt76: mt76x02: wake queues after reconfig Sasha Levin
2026-04-20 13:19 ` Sasha Levin [this message]
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: fix queue pause after scan due to wrong channel switch reason Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 7.0-6.12] net: ethernet: mtk_eth_soc: avoid writing to ESW registers on MT7628 Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 7.0-6.6] wifi: mt76: mt7996: reset device after MCU message timeout Sasha Levin
2026-04-20 13:21 ` [PATCH AUTOSEL 7.0-5.10] drm/mediatek: mtk_dsi: enable hs clock during pre-enable Sasha Levin
2026-04-20 13:21 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: fix frequency separation for station STR mode Sasha Levin
2026-04-20 13:22 ` [PATCH AUTOSEL 6.18] net: airoha: Fix memory leak in airoha_qdma_rx_process() Sasha Levin

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=20260420132314.1023554-186-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=nbd@nbd.name \
    --cc=patches@lists.linux.dev \
    --cc=ryder.lee@mediatek.com \
    --cc=sean.wang@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