[PATCH AUTOSEL 6.18-5.10] wifi: libertas: reject short monitor TX frames

[Sasha Levin]

From: Pengpeng Hou <pengpeng@xxxxxxxxxxx>

[ Upstream commit 13ff543e0b2c713aedeaadadde686686e949dc78 ]

In monitor mode, lbs_hard_start_xmit() casts skb->data to a
radiotap TX header, skips that header, and then copies the 802.11
destination address from offset 4 in the remaining frame.  The
generic length check only rejects zero-length and oversized skbs, so
a short monitor frame can be read past the end of the skb data.

Require enough bytes for the radiotap TX header and the destination
address field before using the monitor-mode header layout.

Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
Link: https://patch.msgid.link/20260704011140.37639-1-pengpeng@xxxxxxxxxxx
Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `wifi: libertas: reject short monitor TX
frames`

**Local tree:** Linux 6.18.44 (`git describe HEAD` →
`v6.18.44-1-g2736c32da98b9`)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
**Record:** `[wifi: libertas]` `[reject]` — Reject undersized monitor-
mode TX frames in the Marvell Libertas wireless driver.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Pengpeng Hou `<pengpeng@xxxxxxxxxxx>` (author)
- **Signed-off-by:** Johannes Berg `<johannes.berg@xxxxxxxxx>` (wireless
  maintainer acceptance)
- **Link:**
  `https://patch.msgid.link/20260704011140.37639-1-pengpeng@xxxxxxxxxxx`
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, or Cc:
  stable tags
- Notable: maintainer SOB from Johannes Berg; no syzbot/fuzzer report

### Step 1.3: Body Analysis
**Record:**
- **Bug:** In monitor mode, `lbs_hard_start_xmit()` treats `skb->data`
  as a radiotap TX header, skips it, then copies the 802.11 destination
  address from offset 4. The existing length check only rejects zero-
  length and oversized SKBs.
- **Symptom:** Short monitor-mode frames cause reads past the end of skb
  data (out-of-bounds access).
- **Root cause:** Missing minimum-length validation for the monitor-mode
  header layout before dereferencing/copying.
- **Version info:** None stated in the commit message.

### Step 1.4: Hidden Bug Fix Detection
**Record:** Not disguised — this is an explicit bounds-check bug fix,
though described without words like "overflow" or "OOB."

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
**Record:**
- **Files:** `drivers/net/wireless/marvell/libertas/tx.c` (+7 lines)
- **Function:** `lbs_hard_start_xmit()`
- **Scope:** Single-file, surgical fix inside the
  `NL80211_IFTYPE_MONITOR` branch

### Step 2.2: Code Flow Change
**Record:**
- **Before:** After generic `skb->len` check (reject 0 or > max),
  monitor path immediately reads `rtap_hdr->rate`, advances past
  `sizeof(*rtap_hdr)`, and `memcpy()`'s 6 bytes from `p802x_hdr + 4`.
- **After:** Same path, but first verifies `skb->len >=
  sizeof(*rtap_hdr) + 4 + ETH_ALEN` (22 bytes with `tx_radiotap_hdr` =
  12 bytes). On failure: log, increment drop/error stats, `goto free`.
- **Path affected:** Monitor-mode TX only; normal (802.3) TX unchanged.

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Buffer overflow / out-of-bounds read
- **Mechanism:**
  1. `rtap_hdr->rate` read with `skb->len < 12` → OOB read
  2. `memcpy(..., p802x_hdr + 4, ETH_ALEN)` with insufficient data → OOB
     read
  3. `pkt_len -= sizeof(*rtap_hdr)` when `skb->len < sizeof(*rtap_hdr)`
     → `uint16_t` underflow → `memcpy(&txpd[1], p802x_hdr, pkt_len)` at
     line 143 can attempt a very large copy → severe OOB read/write

### Step 2.4: Fix Quality
**Record:**
- Length-check logic is correct: `sizeof(*rtap_hdr) + 4 + ETH_ALEN`
  covers radiotap header + 802.11 FC field (4 bytes) + destination
  address (6 bytes).
- Minimal, matches existing error-handling style (stats + `goto free`).
- **Concern:** The new check runs *after* `spin_unlock_irqrestore()` at
  line 106 and after `priv->tx_pending_len = -1` at line 105. A `goto
  free` from there reaches the `free:` label without re-acquiring
  `driver_lock`, yet `unlock:` always calls `spin_unlock_irqrestore()`.
  This is inconsistent with early `goto free` paths (lines 78–88) that
  hold the lock. On the error path, `tx_pending_len` would also remain
  `-1` and queues remain stopped. The length check would be safer before
  line 92 (while lock is held, before queue stop). The core bounds-check
  logic is sound; error-path cleanup placement is suboptimal.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
**Record:** Shallow clone (`git rev-parse --is-shallow-repository` →
`true`) limits blame depth. Blame on lines 117–128 points to merge
commit `5d324e5159d9e` only. Monitor-mode TX code with `tx_radiotap_hdr`
is present in this 6.18.44 tree.

### Step 3.2: Fixes: Tag
**Record:** N/A — no Fixes: tag in commit message.

### Step 3.3: File History
**Record:** `git log --oneline --
drivers/net/wireless/marvell/libertas/tx.c` returns only one shallow
merge entry. Recent libertas stable-style fixes visible in shallow
history include UAF, memory leak, and URB fixes (`ed7d30f90b77f`,
`6cda91bbb8dc3`, etc.), indicating this driver does receive stable
backports.

### Step 3.4: Author History
**Record:** Pengpeng Hou has other bounds-check fixes in this repo
(e.g., CAN drivers). Johannes Berg is the wireless subsystem maintainer
(Signed-off-by).

### Step 3.5: Dependencies
**Record:** Standalone single-patch fix. Uses `struct tx_radiotap_hdr`
from `radiotap.h` and `ETH_ALEN` — both present in this tree. No series
dependencies.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original Discussion
**Record:** Fetched via curl from `https://lore.kernel.org/linux-
wireless/20260704011140.37639-1-pengpeng@xxxxxxxxxxx/t.mbox.gz`.
Original submission only; no reply thread visible in mbox. URL:
https://lore.kernel.org/linux-
wireless/20260704011140.37639-1-pengpeng@xxxxxxxxxxx/

### Step 4.2: Reviewers
**Record:** CC'd: `linux-wireless@xxxxxxxxxxxxxxx`, `libertas-
dev@xxxxxxxxxxxxxxxxxxx`, `linux-kernel@xxxxxxxxxxxxxxx`. Johannes Berg
Signed-off-by indicates maintainer acceptance. `b4 dig` could not be run
(no commit hash in this shallow tree).

### Step 4.3: Bug Report
**Record:** No external bug report, syzbot link, or user Reported-by.
Bug identified through code analysis by the author.

### Step 4.4: Related Patches
**Record:** Standalone; not part of a series.

### Step 4.5: Stable List History
**Record:** No stable-list discussion found for this specific patch
(lore search returned empty).

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key Functions
**Record:** `lbs_hard_start_xmit()` modified.

### Step 5.2: Callers
**Record:** Registered as `.ndo_start_xmit` in:
- `drivers/net/wireless/marvell/libertas/main.c` (line 809)
- `drivers/net/wireless/marvell/libertas/mesh.c` (line 968)

Standard netdev TX hot path — invoked when userspace/kernel transmits on
the Libertas interface.

### Step 5.3: Callees
**Record:** On monitor path: `convert_radiotap_rate_to_mv()`,
`memcpy()`, `lbs_mesh_set_txpd()`, further `memcpy(&txpd[1], ...)`.
Error path: `dev_kfree_skb_any()`, `spin_unlock_irqrestore()`,
`wake_up()`.

### Step 5.4: Reachability
**Record:**
- Monitor mode enabled when `lbs_rtap_supported(priv)` is true (`cfg.c`
  line 2168–2169).
- Requires `CONFIG_LIBERTAS` + USB/SDIO/SPI transport.
- Userspace with `CAP_NET_ADMIN` can set monitor mode and inject TX
  frames (e.g., via `packet_socket` / monitor interfaces).
- Bug reachable from userspace on affected hardware, though hardware
  population is small (Marvell Libertas 8385/8388/8686 — OLPC-era and
  legacy USB/SDIO devices).

### Step 5.5: Similar Patterns
**Record:** Non-monitor path at line 131 also copies `ETH_ALEN` bytes
without a minimum-length check (pre-existing, separate issue). Monitor
path is uniquely vulnerable due to the additional radiotap header and
offset-4 802.11 address extraction.

---

## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE

### Step 6.1: Buggy Code Present?
**Record:** **YES.** Current `tx.c` in 6.18.44 lacks the length check.
Monitor-mode TX path at lines 117–128 matches the pre-fix code exactly.
Monitor mode support is present (`cfg.c`, `cmd.c`, `rx.c`,
`radiotap.h`).

### Step 6.2: Backport Complications
**Record:** **Clean apply expected.** No recent refactoring of this
function visible in shallow history. Single hunk, 7 lines.

### Step 6.3: Related Fixes Already Present?
**Record:** No existing fix for short monitor TX frames found. Other
libertas stability fixes (UAF, leaks) are in history but unrelated.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem Criticality
**Record:** `drivers/net/wireless/marvell/libertas` — **PERIPHERAL**
driver (legacy Marvell WLAN hardware). Config-dependent
(`CONFIG_LIBERTAS`).

### Step 7.2: Subsystem Activity
**Record:** Low activity but receives occasional stability fixes.
Monitor mode is mature (firmware command `CMD_802_11_MONITOR_MODE` in
`host.h`, OLPC-era comment).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who Is Affected
**Record:** Users of Marvell Libertas hardware (USB/SDIO/SPI) running
monitor mode with packet injection. Small but real user population (OLPC
XO, legacy dongles).

### Step 8.2: Trigger Conditions
**Record:** TX of a monitor-mode frame shorter than 22 bytes. Unlikely
in normal operation but trivially triggerable by crafted userspace
packets. Requires monitor mode (typically `CAP_NET_ADMIN`).

### Step 8.3: Failure Mode Severity
**Record:** Out-of-bounds read; potential `uint16_t` underflow leading
to large `memcpy` OOB. **Severity: HIGH** when triggered (kernel memory
safety violation, possible crash under KASAN, potential info leak).
Without fix, every short injected frame hits this path.

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** Prevents OOB access on netdev TX path for monitor-mode
  injection — standard stable-worthy bug class.
- **Risk:** Very low for normal traffic (only rejects invalid short
  frames). The `goto free` placement after `spin_unlock` is a minor
  concern on the error-only path; stable maintainers may want to move
  the check earlier during backport.
- **Ratio:** Benefit outweighs risk for this tree.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Summary

**FOR backport:**
- Real, verifiable OOB read bug in TX path
- Small (7-line), contained, single-file fix
- Buggy code confirmed present in Linux 6.18.44
- Wireless maintainer (Johannes Berg) Signed-off-by
- Similar libertas stability fixes have been backported before
- No new APIs or features
- Triggerable from userspace on affected hardware

**AGAINST backport:**
- Niche hardware (limited user base)
- No syzbot/fuzzer report or user bug report
- Error-path `goto free` after `spin_unlock` is suboptimal (lock state /
  `tx_pending_len` cleanup concern)
- Shallow clone prevented full history of when monitor TX was introduced

**Unresolved:**
- Exact kernel version that introduced monitor TX path (shallow clone)
- Whether mainline adjusted error-path placement post-merge

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — bounds check is
   mathematically correct; maintainer SOB; error-path placement is a
   minor concern but fix logic is sound
2. Fixes a real bug affecting users? **PASS** — OOB read on monitor TX
3. Important issue? **PASS** — memory safety / potential crash (HIGH
   severity when triggered)
4. Small and contained? **PASS** — 7 lines, one file
5. No new features or APIs? **PASS**
6. Can apply to local tree? **PASS** — code exists, clean apply expected

### Step 9.3: Exception Categories
**Record:** None (not a device ID, quirk, DT, build, or docs fix).
Standard memory-safety bug fix.

### Step 9.4: Decision Rationale

This commit fixes a genuine out-of-bounds read in the Libertas driver's
monitor-mode transmit path. The buggy code is present in the local
6.18.44 tree. The fix is minimal, adds only a bounds check with existing
error-handling conventions, and addresses a memory-safety issue
reachable from userspace packet injection on affected hardware. While
the hardware is niche, OOB reads in netdev TX handlers are exactly the
kind of fix stable trees accept. The error-path `goto free` placement
after `spin_unlock` is worth noting for backporters but does not negate
the value of the bounds check itself.

---

## Verification

- [Phase 1] Parsed subject, tags (Hou SOB, Berg SOB, Link); no
  Fixes/Reported-by/syzbot
- [Phase 1] Read commit body: OOB read on short monitor TX frames
- [Phase 2] Diff: +7 lines in `lbs_hard_start_xmit()` monitor branch
- [Phase 2] Read full `tx.c`: confirmed pre-fix code at lines 117–128,
  `goto free`/`unlock` flow at lines 161–166
- [Phase 2] Calculated `sizeof(tx_radiotap_hdr)` = 12 bytes; minimum
  check = 22 bytes
- [Phase 2] Identified `pkt_len` uint16_t underflow risk at line 125/143
  without fix
- [Phase 3] `git rev-parse --is-shallow-repository` → true (limited
  history)
- [Phase 3] `git blame -L 117,128 tx.c` → merge commit only
- [Phase 3] `git log --grep=libertas` → prior stable-style libertas
  fixes found
- [Phase 4] curl lore mbox.gz → original patch fetched, no replies in
  thread
- [Phase 4] WebFetch patch.msgid.link and lore → blocked by bot
  protection; curl succeeded
- [Phase 5] `grep lbs_hard_start_xmit` → callers in `main.c`, `mesh.c`
- [Phase 5] Read `cfg.c` lines 2165–2169 → monitor mode gated on
  `lbs_rtap_supported()`
- [Phase 5] Read `radiotap.h` → `struct tx_radiotap_hdr` layout
  confirmed
- [Phase 6] `git describe HEAD` → v6.18.44; `Makefile` → 6.18.44
- [Phase 6] Read current `tx.c` → buggy code present, fix NOT yet
  applied
- [Phase 6] Read `Kconfig` → `CONFIG_LIBERTAS` dependencies confirmed
- [Phase 8] Assessed trigger: monitor mode + short TX frame from
  userspace
- [Phase 8] Assessed severity: OOB read / potential large memcpy → HIGH
- UNVERIFIED: Exact commit that introduced monitor TX path (shallow
  clone)
- UNVERIFIED: Whether mainline adjusted `goto free` placement after
  initial submission

**YES**

 drivers/net/wireless/marvell/libertas/tx.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/tx.c b/drivers/net/wireless/marvell/libertas/tx.c
index 27304a98787d6..13d08022e4141 100644
--- a/drivers/net/wireless/marvell/libertas/tx.c
+++ b/drivers/net/wireless/marvell/libertas/tx.c
@@ -117,6 +117,13 @@ netdev_tx_t lbs_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (priv->wdev->iftype == NL80211_IFTYPE_MONITOR) {
 		struct tx_radiotap_hdr *rtap_hdr = (void *)skb->data;
 
+		if (skb->len < sizeof(*rtap_hdr) + 4 + ETH_ALEN) {
+			lbs_deb_tx("tx err: short monitor frame %u\n", skb->len);
+			dev->stats.tx_dropped++;
+			dev->stats.tx_errors++;
+			goto free;
+		}
+
 		/* set txpd fields from the radiotap header */
 		txpd->tx_control = cpu_to_le32(convert_radiotap_rate_to_mv(rtap_hdr->rate));
 
-- 
2.53.0