[PATCH AUTOSEL 6.18-6.6] nvmem: brcm_nvram: fix out-of-bounds access on malformed flash data

[Sasha Levin]

From: Rosen Penev <rosenp@xxxxxxxxx>

[ Upstream commit a67e2c323a47a2254ed3472b5e4c53abac20f116 ]

The length check in brcm_nvram_parse() validated header->len against
priv->nvmem_size (the full partition size) instead of priv->data_len
(the actual allocated data buffer). A malformed flash partition with
header->len between the two would pass the check, causing
brcm_nvram_add_cells() to read and write priv->data[len - 1] beyond
the heap allocation.

Also add a minimum bound: len < sizeof(*header) could underflow the
data[len - 1] access.

Fix both bounds by rejecting len outside [sizeof(*header), priv->data_len].

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
Signed-off-by: Srinivas Kandagatla <srini@xxxxxxxxxx>
Link: https://patch.msgid.link/20260729094647.111468-14-srini@xxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

**YES for this 6.6.y tree.** This is a small heap out-of-bounds fix in
`brcm_nvram_parse()`, and the buggy check is present in this checkout.

Local tree: `pending-6.6`, `6.6.157` (`p-6.6-3-g04810563abb71`).
Upstream commit `a67e2c323a47a` is **not** an ancestor of HEAD.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[nvmem / brcm_nvram] [fix] [OOB access when NVRAM header length
is malformed]`

**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Assisted-by: opencode:big-pickle` (AI-assisted)
- `Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>` (author)
- `Signed-off-by: Srinivas Kandagatla <srini@xxxxxxxxxx>` (nvmem
  maintainer)
- `Link:
  https://patch.msgid.link/20260729094647.111468-14-srini@xxxxxxxxxx`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>`
  (char-misc merge)
- No `Fixes:`, no `Reported-by:`, no `Tested-by:`, no `Reviewed-by:` on
  this patch itself, no `Cc: stable` (absence is expected)

**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: `brcm_nvram_parse()` checks `header->len` against
  `priv->nvmem_size` (full mapped partition) instead of `priv->data_len`
  (heap copy).
- Symptom: malformed flash with `data_len < header->len <= nvmem_size`
  passes the check; `brcm_nvram_add_cells()` then reads/writes
  `priv->data[len - 1]` past the allocation.
- Second bug: `len < sizeof(*header)` can underflow that `data[len - 1]`
  index.
- Root cause: wrong upper bound after the driver started caching a
  trimmed copy of NVRAM.
- Version info in the message: none. Locally the mismatch was introduced
  by `09283d60bc332` (“store a copy of NVRAM content”), present since
  **v6.6.9**.

**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. Subject and body describe a real OOB. The 13/14
numbering is the nvmem 7.3 pull, not a hidden refactor.

---

## PHASE 2: DIFF ANALYSIS

**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/nvmem/brcm_nvram.c` (+7 / −3, net +4)
- Function: `brcm_nvram_parse()` only
- Scope: single-file surgical bounds check

**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- Before: after magic check, `len = le32_to_cpu(header->len)`; reject
  only if `len > priv->nvmem_size`; then `brcm_nvram_add_cells(priv,
  priv->data, len)`.
- After: reject if `len < sizeof(*header)` or `len > priv->data_len`.
- Path: probe/parse only (init), not a hot path.

Verified in this tree:

```126:127:drivers/nvmem/brcm_nvram.c
        tmp = priv->data[len - 1];
        priv->data[len - 1] = '\0';
```

```87:91:drivers/nvmem/brcm_nvram.c
        priv->data = devm_kzalloc(priv->dev, priv->data_len,
GFP_KERNEL);
        if (!priv->data)
                return -ENOMEM;

        memcpy_fromio(priv->data, base, priv->data_len);
```

```189:194:drivers/nvmem/brcm_nvram.c
        len = le32_to_cpu(header->len);
        if (len > priv->nvmem_size) {
                dev_err(dev, "NVRAM length (%zd) exceeds mapped size
(%zd)\n", len,
                        priv->nvmem_size);
                return -EINVAL;
```

`data` is allocated to `data_len`, which is the partition with trailing
padding stripped, so `data_len` is typically much smaller than
`nvmem_size` (NAND NVRAM partitions are often 1MiB+; content is usually
16–32KiB). The loops in `brcm_nvram_add_cells()` also walk `data + len`.

**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- Category: **buffer overflow / out-of-bounds** (heap read + write),
  plus underflow if `len == 0`.
- Mechanism: wrong upper bound vs allocated buffer; missing lower bound
  before `data[len - 1]`.
- Fix: reject `len` outside `[sizeof(*header), priv->data_len]`.

**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: the index used is `priv->data[len - 1]` on a
  `data_len`-sized buffer.
- Minimal: one function, one check.
- Regression risk: low. Probe fails with `-EINVAL` instead of heap OOB.
  A theoretical case is well-formed NVRAM whose trailing bytes match
  `padding_byte`, so `data_len < header->len`; those images already OOB
  today. No revert/follow-up on `origin/master` after `a67e2c323a47a`.

---

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: BLAME THE CHANGED LINES**
Record: The `len > priv->nvmem_size` check was added in `09283d60bc332`
(Rafał Miłecki, 2023-12-15), backported here as of **v6.6.9**. That
commit allocated `priv->data` of size `data_len` but left the length
check against `nvmem_size`.

**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. The introducing commit is `09283d60bc332` /
upstream `1e37bf84afacd` (“store a copy of NVRAM content”). `git merge-
base --is-ancestor 09283d60bc332 HEAD` → **yes**. That commit itself had
`Cc: stable` and `Fixes: 3fef9ed0627a`.

**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record (this tree, `drivers/nvmem/brcm_nvram.c`): last functional change
is the store-copy commit. No later local fix for this OOB.
Sibling on mainline (not required for this diff): `fde46579cf887`
“reject empty NVRAM partition” (ZERO_SIZE_PTR / empty partition). It
touches `brcm_nvram_copy_data()`, not `brcm_nvram_parse()`. Independent.
Also **not** in HEAD.

**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Rosen Penev has no other `drivers/nvmem/` commits in this 6.6
tree. On `origin/master` the two brcm_nvram fixes are this OOB and the
empty-partition sibling. Srinivas (nvmem maintainer) SOB’d and sent it
in the 7.3 pull.

**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Standalone. Needs `priv->data` / `priv->data_len` /
`priv->nvmem_size`, all present. 13/14 is pull numbering only. Applies
without 12/14.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c a67e2c323a47a`:
  https://patch.msgid.link/20260729094647.111468-14-srini@xxxxxxxxxx
- `b4 dig -a`: v1 standalone 2026-07-03
  (`20260703222852.410370-1-rosenp@xxxxxxxxx`); then unchanged as 13/14
  in Srinivas’s `[PATCH 00/14] nvmem: updates for 7.3` (2026-07-29).
  Diff did not change.
- No NAK in the saved thread. Cover letter explicitly calls out this
  OOB.
- No reviewer `Cc: stable` on this patch.

**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc were srini, gregkh, linux-kernel, Rosen.
Original v1 also CCd rafal@xxxxxxxxxx. Maintainer SOB is the review
signal; no extra `Reviewed-by` on this patch. Other `Reviewed-by` tags
in the mbox belong to other series patches (e.g. Krzysztof on 14/14
qfprom bindings).

**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla/`Reported-by`. Bug is from code inspection of
the length vs allocation mismatch. Still a reachable probe-time OOB on
real DT platforms.

**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Same author/series: empty-partition reject (12/14). Independent
bug (erased partition → `kzalloc(0)` → fault on `header->magic`). Not a
prerequisite. Pengpeng Hou’s “validate cached NVRAM length” (2026-07-15)
was not merged; this patch was.

**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: No useful stable-list discussion found for this subject. Lore
search was noisy/unrelated; no evidence it was rejected for stable.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `brcm_nvram_parse()` changed; it calls `brcm_nvram_add_cells()`,
which performs the OOB.

**Step 5.2: TRACE CALLERS**
Record: `brcm_nvram_probe()` → `brcm_nvram_copy_data()` then
`brcm_nvram_parse()`. Probe of `compatible = "brcm,nvram"`. Boot/device-
init, not syscall hot path.

**Step 5.3: TRACE CALLEES**
Record: `memcmp` (magic), `le32_to_cpu`, `dev_err`,
`brcm_nvram_add_cells()` (NUL-terminates `data[len-1]`, walks key=value
cells).

**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Platform probe on BCM5301X boards. Userspace cannot call this
directly; corrupted/malformed flash at boot can. Some routers expose
NVRAM to userspace, but that was not verified as a write path into this
partition. Reachable at boot on those devices: **yes**.

**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Sibling empty-partition fix is the other bounds hole in the same
driver. No duplicate of this exact check in this tree (`len >
priv->data_len` is absent).

---

## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE

**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `drivers/nvmem/brcm_nvram.c` `brcm_nvram_parse()` still
uses `priv->nvmem_size`. Introduced in `09283d60bc332` (in tree since
v6.6.9). `git merge-base --is-ancestor a67e2c323a47a HEAD` → **not in
HEAD**.

**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** (offset only: ~189 vs ~192). Pre-image context
matches exactly. `data_len` / `nvmem_size` / `add_cells` `data[len-1]`
all present. No conflicting local edits.

**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No. `git log --grep=out-of-bounds/--grep=malformed --
drivers/nvmem/` is empty. Empty-partition sibling also absent (not
required).

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `drivers/nvmem/` Broadcom NVRAM. **PERIPHERAL / platform-
specific**, but production hardware: `CONFIG_NVMEM_BRCM_NVRAM` depends
on `ARCH_BCM_5301X || COMPILE_TEST`. Kconfig: BCM470X/BCM5301X Northstar
home routers and APs. This tree has **16** `compatible = "brcm,nvram"`
DTS files (Linksys, ASUS, D-Link, Luxul, etc.).

**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: File is mature in 6.6; last local functional change is the 2024
store-copy backport. The OOB is a leftover from that stable backport,
not new mainline-only code.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Driver/platform-specific: BCM5301X routers/APs using
`brcm,nvram`. Not universal.

**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Device probe when the flash header has magic `FLSH` and
`header->len` in `(data_len, nvmem_size]` or `len < sizeof(header)`.
Common on NAND where the partition is much larger than content.
Unprivileged trigger: not a generic syscall; possible with corrupted
NAND or a crafted image. Timing: every boot of an affected board with
bad header.

**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Heap OOB **read and write** (`data[len-1]` plus parse loops).
Crash, memory corruption, or worse. Severity: **HIGH** (platform-limited
**CRITICAL** memory safety).

**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- Benefit: **high** for BCM5301X 6.6 users (stops heap OOB on probe).
- Risk: **very low** (7 lines, reject-only, no locking/API change).
- Ratio: strongly in favor of backport.

---

## PHASE 9: FINAL SYNTHESIS

**Step 9.1: COMPILE THE EVIDENCE**

FOR:
- Real heap OOB read/write in this tree
- Bug came in with a commit already backported to 6.6.9
  (`09283d60bc332`)
- Small, obviously correct bounds fix
- Applies cleanly
- Maintainer SOB + merged to mainline
- Shipping Northstar router hardware
- No follow-up revert

AGAINST:
- No `Reported-by` / syzbot
- Platform-specific (`ARCH_BCM_5301X`)
- AI-assisted commit message
- 13/14 of a larger pull (other patches are unrelated refactors)

UNRESOLVED:
- No public tester beyond maintainer merge
- Could not fetch lore.kernel.org directly (403); used `b4 dig`,
  openwall/lkml mirrors, and a saved mbox
- Unverified whether userspace on these routers can plant a malicious
  header; boot-time malformed flash is enough

**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — bound matches allocation; in
   mainline since 2026-07-31 with no revert
2. Fixes a real bug that affects users? **PASS** — OOB on probe with
   bad/corrupt NVRAM
3. Important issue? **PASS** — heap OOB write / memory corruption /
   crash
4. Small and contained? **PASS** — 7 insertions / 3 deletions, one
   function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — context present, clean apply
   with offset

**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not an ID/quirk/DT/docs exception. Ordinary memory-safety bug
fix.

**Step 9.4: MAKE YOUR DECISION**
Backport. This is the missing bound from the store-copy change already
in 6.6.y. Sibling empty-partition patch is a separate crash and is not
required for this one.

---

## What the commit solves for stable users

On Broadcom Northstar boards, NVRAM is copied into a heap buffer of size
`data_len` (padding stripped). The parser still trusted `header->len` up
to the **full partition** size. A header whose length sits between those
values makes `brcm_nvram_add_cells()` write past the allocation. The
extra `len < sizeof(*header)` check stops `data[len - 1]` from wrapping.
For 6.6.y this closes a hole left by the v6.6.9 store-copy backport.

Risk vs benefit: a 4-line check vs heap corruption at boot. Benefit
wins.

---

## Verification

- **[Phase 1]** Parsed tags from `git log -1 a67e2c323a47a`: SOB
  Rosen/Srinivas/Greg; Link msgid; no Fixes/Reported-by/Cc:stable
- **[Phase 2]** Diff: 10-line change in `brcm_nvram_parse()`; local
  `add_cells` uses `priv->data[len - 1]`; `devm_kzalloc(...,
  priv->data_len)`
- **[Phase 2]** Local `brcm_nvram_parse()` still has `if (len >
  priv->nvmem_size)`
- **[Phase 3]** `git blame -L 175,200`: check from `09283d60bc332`; `git
  merge-base --is-ancestor 09283d60bc332 HEAD` → yes; `git describe
  --contains 09283d60bc332` → `v6.6.9~19`
- **[Phase 3]** `git merge-base --is-ancestor a67e2c323a47a HEAD` →
  **not in this tree**
- **[Phase 3]** Sibling `fde46579cf887` also not in HEAD; different
  function
- **[Phase 4]** `b4 dig -c a67e2c323a47a`: lore URL for 13/14; `-a`: v1
  2026-07-03 then pull 2026-07-29, same diff; `-w`:
  srini/gregkh/lkml/Rosen; `-m`: no NAK on this patch
- **[Phase 4]** Cover letter “nvmem: updates for 7.3” names this OOB;
  Pengpeng Hou alternative not merged
- **[Phase 4]** lore.kernel.org WebFetch 403; used b4/openwall/lkml
  instead
- **[Phase 5]** Callers: `brcm_nvram_probe` only (`git grep`)
- **[Phase 6]** `git describe HEAD` → `p-6.6-3-g04810563abb71`; `make
  kernelversion` → `6.6.157`
- **[Phase 6]** Exact pre-image string present in local file (Python
  check)
- **[Phase 7]** Kconfig `depends on ARCH_BCM_5301X`; 16 DTS files with
  `brcm,nvram`
- **[Phase 8]** Failure mode: heap OOB write in `brcm_nvram_add_cells`,
  severity HIGH
- **UNVERIFIED**: userspace-controlled flash header; live tester for
  this exact patch

The fix is small, surgical, already in mainline, and matches the code in
this 6.6 tree.

**YES**

 drivers/nvmem/brcm_nvram.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/nvmem/brcm_nvram.c b/drivers/nvmem/brcm_nvram.c
index 4b3781a70222a..5ec2ceefb77f6 100644
--- a/drivers/nvmem/brcm_nvram.c
+++ b/drivers/nvmem/brcm_nvram.c
@@ -192,9 +192,13 @@ static int brcm_nvram_parse(struct brcm_nvram *priv)
 	}
 
 	len = le32_to_cpu(header->len);
-	if (len > priv->nvmem_size) {
-		dev_err(dev, "NVRAM length (%zd) exceeds mapped size (%zd)\n", len,
-			priv->nvmem_size);
+	if (len < sizeof(*header)) {
+		dev_err(dev, "NVRAM length (%zd) too small\n", len);
+		return -EINVAL;
+	}
+	if (len > priv->data_len) {
+		dev_err(dev, "NVRAM length (%zd) exceeds data size (%zd)\n", len,
+			priv->data_len);
 		return -EINVAL;
 	}
 
-- 
2.53.0