[PATCH AUTOSEL 6.19-5.10] wifi: brcmfmac: validate bsscfg indices in IF events

[Sasha Levin <sashal@kernel.org>]

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

[ Upstream commit 304950a467d83678bd0b0f46331882e2ac23b12d ]

brcmf_fweh_handle_if_event() validates the firmware-provided interface
index before it touches drvr->iflist[], but it still uses the raw
bsscfgidx field as an array index without a matching range check.

Reject IF events whose bsscfg index does not fit in drvr->iflist[]
before indexing the interface array.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Link: https://patch.msgid.link/20260323074551.93530-1-pengpeng@iscas.ac.cn
[add missing wifi prefix]
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed for a complete analysis. Let me
compile the findings.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: Subject Line**
- Subsystem: `wifi: brcmfmac`
- Action verb: "validate" — this is adding a missing safety check
- Summary: Adds bounds validation for firmware-provided `bsscfgidx`
  before using it as an array index
- Record: [wifi: brcmfmac] [validate] [add missing bounds check for
  bsscfgidx in IF event handler]

**Step 1.2: Tags**
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` — author
- `Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>` — Broadcom
  subsystem maintainer acked it
- `Link:
  https://patch.msgid.link/20260323074551.93530-1-pengpeng@iscas.ac.cn`
  — original submission
- `Signed-off-by: Johannes Berg <johannes.berg@intel.com>` — applied by
  wireless maintainer
- No Fixes: tag, no Cc: stable, no Reported-by — expected for manual
  review candidates

**Step 1.3: Commit Body**
- The commit message explains that `brcmf_fweh_handle_if_event()`
  already validates `ifidx` before using it to index `drvr->iflist[]`,
  but does NOT validate `bsscfgidx` before using it the same way.
- Bug: out-of-bounds array access when firmware provides `bsscfgidx >=
  BRCMF_MAX_IFS` (16)
- Failure mode: kernel crash / memory corruption / potential code
  execution

**Step 1.4: Hidden Bug Fix?**
- This IS a bug fix. The word "validate" means adding a missing safety
  check. The commit directly prevents an out-of-bounds array access from
  firmware-controlled data.

## PHASE 2: DIFF ANALYSIS

**Step 2.1: Inventory**
- Single file: `drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c`
- +5 lines added, 0 removed
- Function modified: `brcmf_fweh_handle_if_event()`
- Scope: single-file surgical fix

**Step 2.2: Code Flow Change**
- BEFORE: `ifevent->bsscfgidx` is used directly as an array index at
  line 157 (`ifp = drvr->iflist[ifevent->bsscfgidx]`) and at line 162
  (`brcmf_add_if(drvr, ifevent->bsscfgidx, ...)`), with no bounds check.
- AFTER: A bounds check `ifevent->bsscfgidx >= BRCMF_MAX_IFS` is added
  before any array access, returning early with an error log if out of
  bounds.
- Only affects the error path for malformed IF events from firmware.

**Step 2.3: Bug Mechanism**
- Category: **Buffer overflow / out-of-bounds array access**
- `drvr->iflist` is declared as `struct brcmf_if *iflist[BRCMF_MAX_IFS]`
  where `BRCMF_MAX_IFS = 16`
- `ifevent->bsscfgidx` is a `u8` from firmware (range 0-255), used as
  index without validation
- This is the SAME class of bug as **CVE-2022-3628**, which was fixed in
  the sibling code path by commit `6788ba8aed4e2`

**Critical insight**: There are TWO separate `bsscfgidx` fields from
firmware:
1. `event->emsg.bsscfgidx` (event message header) — validated by
   CVE-2022-3628 fix in `brcmf_fweh_event_worker()`
2. `ifevent->bsscfgidx` (IF event payload, `struct brcmf_if_event`) —
   **NOT validated** until this commit

For IF events (BRCMF_E_IF), `brcmf_fweh_event_worker()` calls
`brcmf_fweh_handle_if_event()` which uses `ifevent->bsscfgidx` from the
payload, bypassing the header check entirely.

Additionally, `brcmf_add_if()` at `core.c:860` also indexes
`drvr->iflist[bsscfgidx]` without its own bounds check, so the fix
protects that path too.

**Step 2.4: Fix Quality**
- Obviously correct: identical pattern to `ifidx` check 4 lines above
  and `bsscfgidx` check in `brcmf_fweh_event_worker()` at line 275
- Minimal and surgical: 5 lines, one conditional, one log message, one
  return
- Zero regression risk: only rejects malformed firmware events, does not
  change any normal path

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: Blame**
- The unchecked `drvr->iflist[ifevent->bsscfgidx]` at line 157 was
  introduced by commit `37a869ec859308` (Hante Meuleman, 2015-10-29),
  "brcmfmac: Use consistent naming for bsscfgidx." This was a rename of
  an even older pattern.
- The buggy code has existed since at least 2015, present in ALL active
  stable trees.

**Step 3.2: CVE-2022-3628 (Related Fix)**
- Commit `6788ba8aed4e2` fixed the same bug class (CVE-2022-3628) in
  `brcmf_fweh_event_worker()` but missed the
  `brcmf_fweh_handle_if_event()` path.
- That fix was explicitly Cc'd to stable and applied to all active
  stable trees.
- This current commit completes the original CVE fix.

**Step 3.3: File History**
- The file has minimal recent changes. The last relevant bug fix was
  `6788ba8aed4e2` (the CVE fix). No other patches touch this function.
- This commit is standalone — no dependencies.

**Step 3.4: Author**
- Pengpeng Hou also submitted other security-related bounds-check fixes
  (NFC pn533, bnxt_en, ipv6 ioam6, Bluetooth btusb).

**Step 3.5: Dependencies**
- No dependencies. The check pattern already exists for `ifidx` in the
  same function. Applies cleanly to any tree that has the original code.

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1-4.2: Patch Discussion**
- Acked by Arend van Spriel (Broadcom brcmfmac maintainer) — no caveats
- Applied by Johannes Berg (wireless maintainer) with only a minor
  subject prefix fixup
- Appears to be accepted on v1 (no version indicator in subject)

**Step 4.3: Bug Report**
- No external bug report — this was found by code inspection (comparing
  against the CVE-2022-3628 fix)

**Step 4.4-4.5: Related Patches**
- Standalone fix, not part of a series.

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1-5.2: Functions and Callers**
- `brcmf_fweh_handle_if_event()` is called only from
  `brcmf_fweh_event_worker()` (line 303)
- `brcmf_fweh_event_worker()` is a work queue handler processing
  firmware events
- The firmware events come from the Broadcom WiFi hardware/firmware via
  the event queue

**Step 5.3-5.4: Call Chain**
- Firmware sends events → `brcmf_fweh_process_event()` queues them →
  `brcmf_fweh_event_worker()` processes → `brcmf_fweh_handle_if_event()`
  handles IF events → unchecked array access
- The path is reachable from firmware-provided data. For USB-attached
  Broadcom WiFi devices, a malicious device could craft events with
  arbitrary bsscfgidx values.

**Step 5.5: Similar Patterns**
- The exact same check already exists in `brcmf_fweh_event_worker()` at
  line 275 (for the header's bsscfgidx). This commit adds the equivalent
  check for the payload's bsscfgidx.

## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS

**Step 6.1: Buggy Code in Stable Trees**
- The unvalidated `drvr->iflist[ifevent->bsscfgidx]` access has existed
  since at least kernel 3.x (original brcmfmac code from 2012-2015).
- All active stable trees (5.15, 6.1, 6.6, 6.12) contain this bug.
- The CVE-2022-3628 fix (6788ba8aed4e2) is present in stable 5.15+, but
  only covered the worker path.

**Step 6.2: Backport Complications**
- Expected: clean apply. The function has been stable, with no
  structural changes since the CVE fix.
- The 5-line patch touches a single, well-isolated location.

**Step 6.3: Related Fixes Already in Stable**
- CVE-2022-3628 fix is in stable but incomplete — it doesn't cover this
  code path.

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1: Subsystem Criticality**
- Subsystem: WiFi driver (brcmfmac — Broadcom FullMAC WiFi)
- Criticality: IMPORTANT — widely used in Raspberry Pi, many laptops,
  USB WiFi dongles
- Broadcom WiFi is one of the most common WiFi chipsets in consumer
  hardware

**Step 7.2: Subsystem Activity**
- Moderate activity — recent vendor-specific event handling rework, but
  the core event handler is mature code.

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1: Affected Users**
- All users with Broadcom brcmfmac WiFi hardware (Raspberry Pi, many
  laptops, USB WiFi adapters)

**Step 8.2: Trigger Conditions**
- Triggered by malicious/buggy firmware sending an IF event with
  `bsscfgidx >= 16`
- For USB WiFi: a malicious USB device can inject arbitrary firmware
  events (physical access attack vector)
- For embedded firmware: a firmware bug could produce out-of-range
  values

**Step 8.3: Failure Mode Severity**
- Out-of-bounds array access on `drvr->iflist[]` (16-element pointer
  array)
- Consequence: kernel crash (GPF/oops), potential memory corruption,
  potential code execution
- Severity: **CRITICAL** (same class as CVE-2022-3628)

**Step 8.4: Risk-Benefit**
- BENEFIT: Very high — prevents kernel crash and potential security
  exploit from firmware-controlled data
- RISK: Very low — 5 lines, identical pattern to existing validated
  checks, zero normal-path impact
- Ratio: Strongly favorable for backporting

## PHASE 9: FINAL SYNTHESIS

**Step 9.1: Evidence Compilation**

FOR backporting:
- Fixes an out-of-bounds array access from firmware-controlled data
  (same class as CVE-2022-3628)
- Completes an incomplete CVE fix
- Extremely small (5 lines), obviously correct, identical pattern to
  existing checks
- Acked by Broadcom maintainer, applied by wireless maintainer
- Bug present in ALL stable trees
- Zero regression risk — only rejects malformed firmware events
- Hardware is widely used (Raspberry Pi, laptops, USB WiFi)

AGAINST backporting:
- No reported real-world trigger (found by code audit)
- No CVE assigned (yet) — but same class as CVE-2022-3628

**Step 9.2: Stable Rules Checklist**
1. Obviously correct and tested? **YES** — identical pattern to existing
   checks, Acked by maintainer
2. Fixes a real bug? **YES** — out-of-bounds array access from firmware
   data
3. Important issue? **YES** — kernel crash / security vulnerability
4. Small and contained? **YES** — 5 lines, single file, single function
5. No new features or APIs? **YES** — pure bug fix
6. Can apply to stable? **YES** — clean apply expected, no dependencies

**Step 9.3: Exception Categories**
- Not an exception category — this is a standard, clear-cut
  security/stability bug fix.

**Step 9.4: Decision**
- Overwhelming evidence supports backporting. This is a surgical 5-line
  fix completing a CVE fix that was already backported to stable.

## Verification

- [Phase 1] Parsed tags: Acked-by from Broadcom maintainer, Link to
  patch submission, applied by Johannes Berg
- [Phase 2] Diff analysis: +5 lines in `brcmf_fweh_handle_if_event()`,
  adds bounds check for `ifevent->bsscfgidx` before `drvr->iflist[]`
  indexing
- [Phase 2] Verified TWO separate bsscfgidx fields: `struct
  brcmf_event_msg_be.bsscfgidx` (fweh.h:234) vs `struct
  brcmf_if_event.bsscfgidx` (fweh.h:283) — confirmed different
  structures, different data sources
- [Phase 2] Verified `brcmf_add_if()` at core.c:860 also uses bsscfgidx
  as array index without its own bounds check — this fix protects that
  path too
- [Phase 3] git blame: line 157 (`drvr->iflist[ifevent->bsscfgidx]`)
  introduced by commit 37a869ec859308 (2015-10-29)
- [Phase 3] git show 6788ba8aed4e2: confirmed CVE-2022-3628 fix only
  added bounds check in `brcmf_fweh_event_worker()`, NOT in
  `brcmf_fweh_handle_if_event()`
- [Phase 3] Verified BRCMF_MAX_IFS = 16, iflist array is 16 elements
  (core.h:118)
- [Phase 4] Patch acked by Arend van Spriel (Broadcom maintainer) with
  no caveats
- [Phase 5] Traced call chain: firmware → brcmf_fweh_process_event() →
  queue → brcmf_fweh_event_worker() → brcmf_fweh_handle_if_event() →
  unchecked array access
- [Phase 5] Confirmed event->emsg.bsscfgidx check at line 275 does NOT
  protect ifevent->bsscfgidx used at line 157 (different data fields)
- [Phase 6] Code exists in all active stable trees (5.15, 6.1, 6.6,
  6.12) — verified via git log
- [Phase 6] CVE-2022-3628 fix (6788ba8aed4e2) is in stable since 5.15,
  but incomplete
- [Phase 8] Failure mode: out-of-bounds read/write on pointer array →
  kernel GPF/oops, severity CRITICAL

**YES**

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index c2d98ee6652f3..1d25dc9ebca8b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -153,6 +153,11 @@ static void brcmf_fweh_handle_if_event(struct brcmf_pub *drvr,
 		bphy_err(drvr, "invalid interface index: %u\n", ifevent->ifidx);
 		return;
 	}
+	if (ifevent->bsscfgidx >= BRCMF_MAX_IFS) {
+		bphy_err(drvr, "invalid bsscfg index: %u\n",
+			 ifevent->bsscfgidx);
+		return;
+	}
 
 	ifp = drvr->iflist[ifevent->bsscfgidx];
 
-- 
2.53.0