[PATCH AUTOSEL 6.18-5.10] wifi: rsi: validate beacon length before fixed buffer copy

Sasha Levin posted 1 patch 3 days, 14 hours ago
drivers/net/wireless/rsi/rsi_91x_hal.c | 8 ++++++++
1 file changed, 8 insertions(+)
[PATCH AUTOSEL 6.18-5.10] wifi: rsi: validate beacon length before fixed buffer copy
Posted by Sasha Levin 3 days, 14 hours ago
From: Pengpeng Hou <pengpeng@iscas.ac.cn>

[ Upstream commit 8ecdeb8b8a33b22c597299043c0dcfce50beb9ea ]

rsi_prepare_beacon() copies the mac80211 beacon frame after
FRAME_DESC_SZ into a management skb whose usable tailroom may be smaller
than MAX_MGMT_PKT_SIZE after alignment.

Validate the beacon length against the actual tailroom before the copy
and skb_put(). Leave ownership of the management skb with the caller on
error, matching the existing rsi_send_beacon() cleanup path.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260705084824.68105-1-pengpeng@iscas.ac.cn
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject Line
**Record:** `[wifi: rsi]` `[validate]` — validate beacon length before
copying into a fixed-size management skb buffer.

### Step 1.2: Tags
**Record:**
- **Fixes:** — absent (expected for manual review)
- **Reported-by:** — absent
- **Tested-by:** — absent
- **Reviewed-by:** — absent
- **Acked-by:** — absent
- **Link:**
  `https://patch.msgid.link/20260705084824.68105-1-pengpeng@iscas.ac.cn`
- **Cc: stable:** — absent
- **Signed-off-by:** Pengpeng Hou `<pengpeng@iscas.ac.cn>` (author);
  Johannes Berg `<johannes.berg@intel.com>` (wireless maintainer merge
  sign-off)

Notable: CC to Kees Cook on the mailing list submission (security
maintainer). No syzbot Reported-by tag.

### Step 1.3: Body Analysis
**Record:**
- **Bug:** `rsi_prepare_beacon()` copies the mac80211 beacon after
  `FRAME_DESC_SZ` (16 bytes) into an skb allocated for
  `MAX_MGMT_PKT_SIZE` (512), but usable tailroom can be smaller after
  dword alignment `skb_pull()` in the caller.
- **Symptom:** Unchecked `memcpy()` + `skb_put()` can write past skb
  buffer end → heap buffer overflow.
- **Version info:** None in commit message.
- **Root cause:** Caller reduces effective buffer space for alignment;
  callee assumes full `MAX_MGMT_PKT_SIZE` is available.

### Step 1.4: Hidden Bug Fix Detection
**Record:** Not disguised — explicitly a bounds-validation fix before
`memcpy()`. Classic buffer-overflow prevention.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **Files:** `drivers/net/wireless/rsi/rsi_91x_hal.c` (+8 / -0)
- **Functions:** `rsi_prepare_beacon()`
- **Scope:** Single-file, surgical fix in one function

### Step 2.2: Code Flow Change
**Record:**
- **Hunk 1 (variable):** Adds `unsigned int tailroom`.
- **Hunk 2 (validation):** Before `memcpy()`:
  - **Before:** Unconditionally copies `mac_bcn->len` bytes and extends
    skb.
  - **After:** Checks `skb_tailroom(skb) >= FRAME_DESC_SZ` and
    `mac_bcn->len <= tailroom - FRAME_DESC_SZ`; on failure frees
    `mac_bcn`, returns `-EMSGSIZE`, leaves caller-owned `skb` untouched.
- **Path affected:** AP/P2P-GO beacon preparation error path (new) and
  success path (unchanged).

### Step 2.3: Bug Mechanism
**Record:** **Category:** Buffer overflow / out-of-bounds write (memory
safety).

**Mechanism:**
1. `rsi_send_beacon()` allocates `dev_alloc_skb(MAX_MGMT_PKT_SIZE)` (512
   bytes).
2. For 64-byte alignment, it may `skb_pull(skb, 64 - dword_align_bytes)`
   — up to 63 bytes, reducing tailroom to as little as ~449 bytes.
3. `rsi_prepare_beacon()` then does `memcpy(&skb->data[FRAME_DESC_SZ],
   mac_bcn->data, mac_bcn->len)` without checking fit.
4. Worst case: safe beacon payload without fix ≈ **433 bytes** (`512 -
   63 - 16`). Beacons with many IEs (HT/VHT/HE, WPS, vendor IEs) can
   exceed this.

### Step 2.4: Fix Quality
**Record:**
- **Quality:** Obviously correct — uses `skb_tailroom()` against actual
  post-alignment space, not the nominal `MAX_MGMT_PKT_SIZE`.
- **Minimal:** 8 lines, no unrelated changes.
- **Error handling:** Correctly frees `mac_bcn` only; caller
  `rsi_send_beacon()` already frees `skb` on any `rsi_prepare_beacon()`
  failure.
- **Regression risk:** Very low. On oversize beacon, AP beacon TX fails
  gracefully instead of corrupting memory.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Lines 483–484 (`memcpy`/`skb_put`) blamed to `5d324e5159d9e`
(Merge tag 'usb-6.18-rc8', 2025-11-28). Local history for this file is
shallow (only 1 commit in `git log --
drivers/net/wireless/rsi/rsi_91x_hal.c`). Exact commit that introduced
the alignment+memcpy pattern is **UNVERIFIED** beyond presence in this
6.18.y tree.

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

### Step 3.3: Related File History
**Record:** Shallow history in this checkout. Related mainline commit
`d06a3e60c8fea` ("wifi: rsi: bound background scan probe request copy")
exists in repo but is **not** an ancestor of HEAD — separate bounds-
check fix, not a prerequisite for this patch.

### Step 3.4: Author Context
**Record:** Pengpeng Hou submitted security-oriented bounds checks for
the RSI driver. Johannes Berg (wireless maintainer) merged. Author
relationship to subsystem: contributor doing targeted hardening.

### Step 3.5: Dependencies
**Record:** Standalone. No patch series markers. No new structures/APIs.
Applies cleanly to current `rsi_91x_hal.c` in this tree.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
**Record:**
- `b4 dig` failed in this environment.
- Openwall archive: https://lists.openwall.net/linux-
  kernel/2026/07/05/209
- Patchew:
  https://patchew.org/linux/20260705084824.68105-1-pengpeng@iscas.ac.cn/
- lore.kernel.org blocked by bot protection.
- **Series revisions:** v1 only (Syzbot CI confirms Version 1).
- **Reviewer feedback:** No replies/NAKs found in accessible archives.
- **Stable nomination:** None found.

### Step 4.2: Reviewers
**Record:** CC'd to `kees@kernel.org`, `linux-wireless@vger.kernel.org`,
`linux-kernel@vger.kernel.org`. Merged with Signed-off-by from Johannes
Berg.

### Step 4.3: Bug Report
**Record:** No external bug report or syzbot crash report. Syzbot CI
tested the patch series and reported "All tests passed" — validation
testing, not a fuzzer-found crash report.

### Step 4.4: Related Patches
**Record:** Same author has a related RSI bounds-check patch for
background scan probe requests on mainline; independent of this fix.

### Step 4.5: Stable List History
**Record:** No stable-list discussion found (UNVERIFIED beyond search
results).

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `rsi_prepare_beacon()` (modified); callers:
`rsi_send_beacon()` only.

### Step 5.2: Callers
**Record:**
- `rsi_send_beacon()` → `rsi_prepare_beacon()` (`rsi_91x_mgmt.c:1777`)
- `rsi_send_beacon()` called from `BEACON_EVENT_IND` case
  (`rsi_91x_mgmt.c:2225`) when AP beaconing is enabled and FSM is in
  `FSM_MAC_INIT_DONE`

### Step 5.3: Callees
**Record:** `ieee80211_beacon_get_tim()`, `dev_kfree_skb()`, `memcpy()`,
`skb_put()`, `skb_tailroom()` (added).

### Step 5.4: Reachability
**Record:**
- Triggered by firmware beacon events on RSI hardware in AP/P2P-GO mode.
- Beacon content comes from mac80211 (host configuration — SSID, IEs,
  security, etc.).
- Not directly a syscall path, but reachable from normal AP operation
  with legitimately large beacon frames.
- Unprivileged users on the AP host can influence beacon size via
  network configuration.

### Step 5.5: Similar Patterns
**Record:** Same driver already bounds-checks management frames
elsewhere (`rsi_91x_hal.c:71` drops pkts `> MAX_MGMT_PKT_SIZE`; `:82-86`
checks headroom). The beacon path was missing the equivalent tailroom
check after alignment — inconsistent and buggy.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
**Record:** **YES.** Local tree is **v6.18.44** (`VERSION=6,
PATCHLEVEL=18, SUBLEVEL=44`). `rsi_prepare_beacon()` at lines 483–484
performs unchecked `memcpy()`/`skb_put()`. Mainline fix commit
`8ecdeb8b8a33b` exists in object DB but is **NOT** an ancestor of HEAD —
fix not yet in this tree.

### Step 6.2: Backport Complications
**Record:** **Clean apply expected.** Target code matches the patch
base. No conflicting changes in this function. Single hunk insertion.

### Step 6.3: Related Fixes Already Present?
**Record:** No `skb_tailroom` or `-EMSGSIZE` usage in RSI driver. No
duplicate fix found.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem
**Record:** `drivers/net/wireless/rsi/` — Redpine Signals 91x WLAN
driver (`CONFIG_RSI_91X`). **Criticality: PERIPHERAL** (hardware-
specific), but memory-safety bug class is kernel-wide in severity.

### Step 7.2: Activity
**Record:** Driver present and functional in 6.18.y. Recent mainline
hardening activity from same author suggests active security review of
this driver.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users with RSI 91x hardware (USB/SDIO modules) running AP or
P2P-GO mode. Config-dependent on `CONFIG_RSI_91X` and bus variants.

### Step 8.2: Trigger Conditions
**Record:**
- AP mode with beacon enabled.
- Firmware sends `BEACON_EVENT_IND`.
- Beacon frame + 16-byte descriptor exceeds post-alignment tailroom.
- Alignment pull is address-dependent (up to 63 bytes); not every
  allocation hits worst case, but it will occur in practice.
- **Unprivileged trigger:** Indirectly yes — AP operator can configure
  beacon IEs that push frame size over the safe threshold.

### Step 8.3: Failure Mode Severity
**Record:** **Heap buffer overflow** past skb allocation → memory
corruption, kernel oops/panic, potential security impact. **Severity:
HIGH** (could be CRITICAL depending on exploitability; at minimum causes
crashes/data corruption).

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** Prevents real memory corruption on AP beacon TX path for
  RSI hardware users.
- **Risk:** Very low — 8-line bounds check on error path only.
- **Ratio:** Strongly favors backport.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real, verifiable buffer overflow in kernel code
- Small, obviously correct fix (+8 lines, 1 file)
- Bug exists in v6.18.44 tree; fix not yet merged
- Merged by wireless maintainer; CC'd to security maintainer
- Consistent with existing bounds-check patterns in same driver
- Graceful failure path already exists in caller

**AGAINST backport:**
- Affects niche hardware (RSI 91x) only
- Requires AP mode with sufficiently large beacons
- No user crash reports or syzbot-found reproducer
- On failure, AP beacon TX stops (functional degradation vs. crash) —
  still far better than overflow

**UNRESOLVED:**
- Exact commit that introduced the alignment-without-bounds-check
  pattern (shallow history)
- Whether reviewers explicitly nominated for stable (no discussion
  found)

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — logic is straightforward;
   Syzbot CI passed on patch.
2. Fixes a real bug affecting users? **PASS** — buffer overflow on AP
   beacon path.
3. Important issue? **PASS** — memory corruption / potential crash or
   security issue.
4. Small and contained? **PASS** — 8 lines, 1 file, 1 function.
5. No new features or APIs? **PASS** — defensive bounds check only.
6. Can apply to local tree? **PASS** — buggy code present, fix absent,
   clean apply.

### Step 9.3: Exception Categories
**Record:** None of the automatic exception categories (device ID,
quirk, DT, build fix, docs). This is a straight memory-safety bug fix.

### Step 9.4: Decision Rationale

For **v6.18.44**, the buggy code is present: `rsi_send_beacon()` reduces
skb tailroom via alignment `skb_pull()`, then `rsi_prepare_beacon()`
blindly copies the full mac80211 beacon. With beacons larger than ~433
bytes (common for feature-rich APs), this overflows the 512-byte skb.
The fix validates against actual `skb_tailroom()` before `memcpy()`,
matching the driver's existing defensive patterns. The patch is minimal,
self-contained, and prevents kernel memory corruption — exactly the kind
of fix stable trees should carry.

---

## Verification

- **[Phase 1]** Parsed commit message and tags from user-provided diff
  and `git show 8ecdeb8b8a33b`
- **[Phase 1]** Openwall list: CC to Kees Cook confirmed at
  https://lists.openwall.net/linux-kernel/2026/07/05/209
- **[Phase 2]** Read `rsi_prepare_beacon()` and `rsi_send_beacon()` in
  local tree
- **[Phase 2]** Confirmed `FRAME_DESC_SZ=16`, `MAX_MGMT_PKT_SIZE=512` in
  `rsi_main.h` / `rsi_mgmt.h`
- **[Phase 2]** Calculated worst-case safe beacon size: 433 bytes (512 -
  63 - 16)
- **[Phase 3]** `git blame -L 483,484`: lines from 5d324e5159d9e
- **[Phase 3]** `git log -- drivers/net/wireless/rsi/rsi_91x_hal.c`:
  only 1 commit (shallow)
- **[Phase 3]** `git merge-base --is-ancestor 8ecdeb8b8a33b HEAD`: fix
  NOT in current branch
- **[Phase 4]** `b4 dig`: failed
- **[Phase 4]** Openwall/Patchew/Syzbot CI: patch found, v1 only, CI
  passed
- **[Phase 4]** lore.kernel.org: blocked by bot protection
- **[Phase 5]** `grep rsi_prepare_beacon`: single caller
  `rsi_send_beacon()`
- **[Phase 5]** Traced `BEACON_EVENT_IND` → `rsi_send_beacon()` call
  chain
- **[Phase 6]** `git describe HEAD`: v6.18.44-1-g2736c32da98b9
- **[Phase 6]** Confirmed unchecked `memcpy` at lines 483–484 without
  tailroom validation
- **[Phase 6]** `grep skb_tailroom/EMSGSIZE` in rsi/: no matches (fix
  absent)
- **[Phase 7]** Read `drivers/net/wireless/rsi/Kconfig`: RSI 91x driver
  config exists
- **[Phase 8]** Assessed failure mode as heap buffer overflow, severity
  HIGH
- **UNVERIFIED:** Exact introduction commit of buggy alignment logic
  (shallow file history)
- **UNVERIFIED:** Reviewer stable nominations (no accessible thread
  replies)

**YES**The background checks finished and match the earlier analysis:

- **Fix commit `8ecdeb8b8a33b`** is in the repo but is **not** on the
  current branch (`v6.18.44`) — the buggy `memcpy()` path is still
  present.
- The patch should apply cleanly; no further investigation is needed.

**Verdict for this tree: YES** — backport the beacon tailroom validation
fix.

 drivers/net/wireless/rsi/rsi_91x_hal.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c b/drivers/net/wireless/rsi/rsi_91x_hal.c
index 7d26314a3e76a..50f77f3eee92c 100644
--- a/drivers/net/wireless/rsi/rsi_91x_hal.c
+++ b/drivers/net/wireless/rsi/rsi_91x_hal.c
@@ -431,6 +431,7 @@ int rsi_prepare_beacon(struct rsi_common *common, struct sk_buff *skb)
 	struct ieee80211_vif *vif;
 	struct sk_buff *mac_bcn;
 	u8 vap_id = 0, i;
+	unsigned int tailroom;
 	u16 tim_offset = 0;
 
 	for (i = 0; i < RSI_MAX_VIFS; i++) {
@@ -480,6 +481,13 @@ int rsi_prepare_beacon(struct rsi_common *common, struct sk_buff *skb)
 	if (mac_bcn->data[tim_offset + 2] == 0)
 		bcn_frm->frame_info |= cpu_to_le16(RSI_DATA_DESC_DTIM_BEACON);
 
+	tailroom = skb_tailroom(skb);
+	if (tailroom < FRAME_DESC_SZ ||
+	    mac_bcn->len > tailroom - FRAME_DESC_SZ) {
+		dev_kfree_skb(mac_bcn);
+		return -EMSGSIZE;
+	}
+
 	memcpy(&skb->data[FRAME_DESC_SZ], mac_bcn->data, mac_bcn->len);
 	skb_put(skb, mac_bcn->len + FRAME_DESC_SZ);
 
-- 
2.53.0