[PATCH AUTOSEL 6.18-5.15] firmware: arm_scmi: Validate SENSOR_UPDATE payload size

[PATCH AUTOSEL 6.18-5.15] firmware: arm_scmi: Validate SENSOR_UPDATE payload size

[Sasha Levin]

From: Sudeep Holla <sudeep.holla@kernel.org>

[ Upstream commit 32bc5496b48174dbca1f187f710955ee4d9527a1 ]

SENSOR_UPDATE carries one or more sensor readings after the fixed
notification header. The parser derives the expected reading count
from the sensor description, but it did not verify that the received
payload contains those entries before parsing them.

Reject truncated update notifications before reading the variable
array.

Link: https://patch.msgid.link/20260517-scmi_fixes-v1-3-d86daec4defd@kernel.org
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
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: `[firmware: arm_scmi]` `[Validate]` — Validate SENSOR_UPDATE
  notification payload size before parsing variable-length sensor
  readings.

**Step 1.2 — Tags**
- Record:
  - `Link:` https://patch.msgid.link/20260517-scmi_fixes-v1-3-
    d86daec4defd@kernel.org
  - `Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>` (ARM SCMI
    maintainer)
  - `Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>` (SCMI
    maintainer)
  - No `Fixes:`, `Reported-by:`, `Cc: stable@vger.kernel.org`, or syzbot
    tags
  - Part of `[PATCH 3/4]` in series `firmware: arm_scmi: Fix protocol
    parsing and validation`

**Step 1.3 — Body analysis**
- Record:
  - **Bug:** `SCMI_EVENT_SENSOR_UPDATE` notifications carry a fixed
    header plus a variable array of readings. The parser derives
    `readings_count` from the sensor description but never checks that
    `payld_sz` covers those entries.
  - **Symptom:** Truncated notifications are parsed anyway; readings
    beyond the valid payload are read and forwarded to handlers.
  - **Root cause:** Missing minimum and expected payload size validation
    before accessing `p->readings[]`.
  - **Version info:** None in commit message; code has existed since
    SCMI v3.0 sensor notifications (2020).

**Step 1.4 — Hidden bug fix?**
- Record: **Yes.** Despite the neutral “validate” wording, this is a
  real parsing bug fix, not cosmetic cleanup. It prevents out-of-spec
  payload processing.

---

## Phase 2: Diff Analysis

**Step 2.1 — Inventory**
- Record:
  - `drivers/firmware/arm_scmi/sensors.c`: +9 / -1 lines
  - Function modified: `scmi_sensor_fill_custom_report()`
  - Scope: single-file, surgical fix in one `switch` case

**Step 2.2 — Code flow change**
- Record:
  - **Hunk 1 (minimum header check):** Before → reads `p->sensor_id`
    immediately. After → returns early if `payld_sz < sizeof(*p)` (8
    bytes).
  - **Hunk 2 (expected size check):** Before → loops `readings_count`
    times over `p->readings[i]` unconditionally. After → computes
    `expected_sz = sizeof(*p) + readings_count * sizeof(p->readings[0])`
    and breaks if `payld_sz < expected_sz`.
  - **Failure path:** `break` leaves `rep = NULL`; caller logs and skips
    notification handlers.

**Step 2.3 — Bug mechanism**
- Record:
  - **Category:** Memory safety / bounds validation (out-of-bounds read
    of notification payload).
  - **Mechanism:** `scmi_notify()` only enforces an upper bound (`len >
    max_payld_sz`). For `SENSOR_UPDATE`, `max_payld_sz` allows up to 63
    axis readings, but a shorter payload is accepted. The handler then
    reads 16-byte `scmi_sensor_reading_resp` entries beyond the copied
    `payld_sz` bytes. The scratch buffer (`pd->eh`) is pre-allocated to
    max size, so this typically reads stale buffer contents rather than
    faulting — but wrong sensor values are still delivered to consumers.

**Step 2.4 — Fix quality**
- Record:
  - Fix is obviously correct; mirrors the existing fixed-size check on
    `SCMI_EVENT_SENSOR_TRIP_POINT_EVENT` and the variable-size pattern
    in `system.c`.
  - Minimal, no API changes.
  - Regression risk: very low — only rejects malformed/truncated
    notifications that were already being mishandled.

---

## Phase 3: Git History Investigation

**Step 3.1 — Blame**
- Record: `SCMI_EVENT_SENSOR_UPDATE` handler introduced in
  `e3811190acf85` (Cristian Marussi, 2020-11-19, “Add SCMI v3.0 sensor
  notifications”). Bug present since introduction. Present in this tree
  at `drivers/firmware/arm_scmi/sensors.c:1074-1101`.

**Step 3.2 — Fixes: tag**
- Record: Not applicable — no `Fixes:` tag.

**Step 3.3 — Related file history**
- Record:
  - Recent related hardening: `76f89c9547887` (“Harden accesses to the
    sensor domains”), `3b0041f6e10e5` (“Validate
    BASE_DISCOVER_LIST_PROTOCOLS response”) — same class of “don’t trust
    SCMI payload sizes.”
  - Patch 1/4 of the same series is already in this tree:
    `bac3e70c2fb10` (“Read sensor config as 32-bit value”).
  - Patches 2/4 and 4/4 of the series are not yet in this tree; patch
    3/4 is standalone.

**Step 3.4 — Author context**
- Record: Sudeep Holla is the SCMI maintainer. Cristian Marussi is the
  primary SCMI protocol author and reviewed this patch.

**Step 3.5 — Dependencies**
- Record: **Standalone.** Only touches existing
  `SCMI_EVENT_SENSOR_UPDATE` path. No prerequisite commits required
  beyond code already in `linux-6.18.y`.

---

## Phase 4: Mailing List and External Research

**Step 4.1 — Original discussion**
- Record:
  - Lore URL: https://lore.kernel.org/linux-arm-
    kernel/20260517-scmi_fixes-v1-3-d86daec4defd@kernel.org/
  - Series cover (patch 0/4) explains: “The next two patches harden
    notification parsing for variable-sized payloads. BASE_ERROR_EVENT
    and SENSOR_UPDATE both carry counted trailing arrays…”
  - “No functional change is intended for well-formed SCMI responses.”
  - Review reply from Cristian Marussi on patch 3/4 exists in thread
    (Reviewed-by in final commit).

**Step 4.2 — Reviewers**
- Record: CC’d to `Cristian Marussi`, `arm-scmi@vger.kernel.org`,
  `linux-arm-kernel@lists.infradead.org`. Subsystem maintainers were
  included.

**Step 4.3 — Bug report**
- Record: No external bug report or syzbot link. Issue found during
  spec-compliance review per series cover letter.

**Step 4.4 — Series context**
- Record: 4-patch series; patch 3 is independent of patches 2 and 4.
  Patch 1 already backported to this tree, indicating stable maintainers
  already consider the series appropriate for `6.18.y`.

**Step 4.5 — Stable list history**
- Record: No explicit `Cc: stable` nomination found in thread. Not a
  negative signal per instructions.

---

## Phase 5: Code Semantic Analysis

**Step 5.1 — Key functions**
- Record: `scmi_sensor_fill_custom_report()`,
  `scmi_parse_sensor_readings()`

**Step 5.2 — Callers**
- Record:
  - `REVT_FILL_REPORT()` macro in `notify.c:495` called from
    `scmi_process_event_payload()`
  - `scmi_process_event_payload()` called from
    `scmi_events_dispatcher()` workqueue handler
  - Context: process context, SCMI notification worker path

**Step 5.3 — Callees**
- Record: `le32_to_cpu()`, `scmi_parse_sensor_readings()` (reads 16-byte
  unaligned LE64 pairs per axis)

**Step 5.4 — Reachability**
- Record:
  - Triggered when platform firmware sends `SCMI_EVENT_SENSOR_UPDATE`
    notifications
  - Affects ARM/ARM64 systems using SCMI (Juno, NXP i.MX, STM32 MP,
    Neoverse, etc.)
  - Not directly userspace-triggerable, but firmware bugs, transport
    corruption, or spec violations can deliver truncated payloads
  - Downstream consumers include
    `drivers/iio/common/scmi_sensors/scmi_iio.c` (registers for
    `SCMI_EVENT_SENSOR_UPDATE` and copies `readings[]` into IIO buffers)

**Step 5.5 — Similar patterns**
- Record:
  - `SCMI_EVENT_SENSOR_TRIP_POINT_EVENT` already validates `sizeof(*p)
    != payld_sz`
  - `scmi_system_fill_custom_report()` validates `payld_sz !=
    expected_sz`
  - `scmi_reset_fill_custom_report()`,
    `scmi_power_fill_custom_report()`, `scmi_perf_fill_custom_report()`
    all validate payload sizes
  - `SENSOR_UPDATE` was the outlier missing validation

---

## Phase 6: Cross-Reference Against Local Tree

**Step 6.1 — Buggy code exists?**
- Record: **Yes.** Local tree is `stable/linux-6.18.y` at `v6.18.44`.
  Buggy code confirmed at `sensors.c:1082-1098` — no payload size
  validation before parsing readings.

**Step 6.2 — Backport complications**
- Record: **Clean apply expected.** File is present and structure
  matches the diff context exactly. No conflicting refactors in this
  area.

**Step 6.3 — Related fixes already present?**
- Record: Patch 1/4 of same series already backported (`bac3e70c2fb10`).
  This specific SENSOR_UPDATE validation is **not** yet present. No
  duplicate fix found.

---

## Phase 7: Subsystem and Maintainer Context

**Step 7.1 — Subsystem criticality**
- Record: `firmware/arm_scmi` — **IMPORTANT** for ARM embedded/server
  platforms. Sensor notifications feed hwmon/IIO/thermal subsystems.

**Step 7.2 — Subsystem activity**
- Record: Actively maintained; recent commits include protocol
  versioning, sensor domain hardening, and the first patch of this same
  fix series.

---

## Phase 8: Impact and Risk Assessment

**Step 8.1 — Who is affected**
- Record: ARM platforms using SCMI sensor continuous-update
  notifications — embedded, mobile, server BMC paths. Config-dependent
  on `CONFIG_ARM_SCMI` and sensor notification registration.

**Step 8.2 — Trigger conditions**
- Record: Truncated or malformed `SENSOR_UPDATE` notification from SCMI
  firmware. Uncommon in normal operation but possible with buggy
  firmware or corrupted messages. Not unprivileged-userspace-
  triggerable.

**Step 8.3 — Failure mode severity**
- Record:
  - **Failure mode:** Reads beyond valid payload into stale scratch-
    buffer data; incorrect sensor readings propagated to IIO/hwmon
    notifiers.
  - **Severity:** **MEDIUM-HIGH** — data integrity issue in sensor
    reporting path; could affect thermal/power decisions on affected
    platforms. Unlikely to panic (buffer is pre-allocated to max size),
    but wrong data is a real bug.

**Step 8.4 — Risk vs benefit**
- Record:
  - **Benefit:** Prevents bogus sensor data from truncated
    notifications; aligns with existing SCMI validation patterns; same
    series already partially backported.
  - **Risk:** Very low — 9-line validation-only change on error path.
  - **Ratio:** Strong benefit, minimal risk.

---

## Phase 9: Final Synthesis

**Step 9.1 — Evidence summary**

*For backporting:*
- Real bounds-check bug in notification parser since 2020
- Incorrect sensor readings possible on truncated payloads
- Small, obviously correct, maintainer-reviewed fix
- Consistent with prior SCMI validation backports in this tree
  (`3b0041f6e10e5`)
- Patch 1/4 of same series already in `linux-6.18.y`
- Standalone, clean apply to this tree

*Against backporting:*
- No user crash report or syzbot finding
- Requires malformed firmware notification (trusted path)
- Part of a series where patches 2 and 4 are not yet backported (but
  patch 3 is independent)

*Unresolved:*
- No explicit stable nomination in lore thread (not required)

**Step 9.2 — Stable rules checklist**
1. Obviously correct and tested? **PASS** — logic is straightforward;
   reviewed by SCMI maintainer; series patch 1 already in tree with
   multiple Tested-by tags on series.
2. Fixes a real bug? **PASS** — truncated payloads parsed without bounds
   check.
3. Important issue? **PASS** — data integrity in sensor notification
   path (MEDIUM-HIGH).
4. Small and contained? **PASS** — 9 lines, one function, one file.
5. No new features/APIs? **PASS** — validation only.
6. Can apply to local tree? **PASS** — buggy code present, clean apply
   expected.

**Step 9.3 — Exception categories**
- Record: Not a device ID/quirk/DT/build/doc exception. Standard bug
  fix.

**Step 9.4 — Decision rationale**

For `linux-6.18.y` at `v6.18.44`, this commit should be backported. The
`SCMI_EVENT_SENSOR_UPDATE` handler has been missing payload size
validation since 2020, allowing truncated notifications to produce
incorrect sensor readings that flow into IIO consumers. The fix is
minimal, matches established patterns elsewhere in the same driver, was
reviewed by the SCMI maintainer, and belongs to a series whose first
patch is already in this stable tree.

---

## Verification

- [Phase 1] `git describe HEAD` → `v6.18.44`; parsed commit message tags
  and body
- [Phase 2] Read diff and `sensors.c:1050-1108`; confirmed
  `scmi_sensor_reading_resp` is 16 bytes; `sizeof(*p)` is 8 bytes
- [Phase 3] `git blame -L 1074,1101 sensors.c` → bug from
  `e3811190acf85` (2020-11-19)
- [Phase 3] `git show e3811190acf85` → introduced SCMI v3.0 sensor
  notifications
- [Phase 3] `git log --oneline -20 --
  drivers/firmware/arm_scmi/sensors.c` → recent activity confirmed
- [Phase 3] `git log --oneline -1 bac3e70c2fb10` → patch 1/4 already in
  tree
- [Phase 4] Fetched lore thread via `curl .../t.mbox.gz`; confirmed
  series context and Reviewed-by
- [Phase 4] `b4 am 20260517-scmi_fixes-v1-3-d86daec4defd@kernel.org` →
  found thread on lore.kernel.org
- [Phase 5] `grep fill_custom_report notify.c` → call chain through
  `scmi_process_event_payload()`
- [Phase 5] Read `scmi_iio.c:39-55` → consumer copies
  `sensor_update->readings[]` to IIO buffers
- [Phase 5] Read `notify.c:598-611, 759-789` → upper-bound-only check in
  `scmi_notify()`; scratch buffer sized to max
- [Phase 6] Confirmed buggy code at `sensors.c:1082-1098` without fix
- [Phase 6] `git branch` shows `HEAD detached at stable/linux-6.18.y`
- [Phase 6] No existing SENSOR_UPDATE validation fix in tree
- [Phase 8] Assessed failure as stale-data reads → incorrect sensor
  values, not kmalloc overflow (scratch buffer pre-allocated to
  `max_payld_sz`)

**YES****Verdict: YES** — backport to `linux-6.18.y`.

The `SCMI_EVENT_SENSOR_UPDATE` handler in
`drivers/firmware/arm_scmi/sensors.c` parses variable-length sensor
readings without checking that the notification payload is large enough.
Truncated messages can produce incorrect readings that reach IIO/hwmon
consumers.

The fix is small (9 lines), reviewed by SCMI maintainer Cristian
Marussi, and matches validation patterns already used elsewhere in the
driver. Patch 1/4 of the same series is already in this tree; this one
is standalone and should apply cleanly.

 drivers/firmware/arm_scmi/sensors.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index 1be0f89fc2c4b..99233742714c9 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -1074,12 +1074,15 @@ scmi_sensor_fill_custom_report(const struct scmi_protocol_handle *ph,
 	case SCMI_EVENT_SENSOR_UPDATE:
 	{
 		int i;
+		size_t expected_sz;
 		struct scmi_sensor_info *s;
 		const struct scmi_sensor_update_notify_payld *p = payld;
 		struct scmi_sensor_update_report *r = report;
 		struct sensors_info *sinfo = ph->get_priv(ph);
 
-		/* payld_sz is variable for this event */
+		if (payld_sz < sizeof(*p))
+			break;
+
 		r->sensor_id = le32_to_cpu(p->sensor_id);
 		if (r->sensor_id >= sinfo->num_sensors)
 			break;
@@ -1093,6 +1096,11 @@ scmi_sensor_fill_custom_report(const struct scmi_protocol_handle *ph,
 		 * readings defined for this sensor or 1 for scalar sensors.
 		 */
 		r->readings_count = s->num_axis ?: 1;
+		expected_sz = sizeof(*p) + r->readings_count *
+			      sizeof(p->readings[0]);
+		if (payld_sz < expected_sz)
+			break;
+
 		for (i = 0; i < r->readings_count; i++)
 			scmi_parse_sensor_readings(&r->readings[i],
 						   &p->readings[i]);
-- 
2.53.0

[PATCH AUTOSEL 6.18-5.10] firmware: arm_scmi: Validate BASE_ERROR_EVENT payload size

[Sasha Levin]

From: Sudeep Holla <sudeep.holla@kernel.org>

[ Upstream commit 56e7e64cdd0e7209a58c8ec66028d63387402919 ]

BASE_ERROR_EVENT carries a variable number of message reports,
with the count encoded in error_status. The notification parser used
that count without checking whether the received payload contained all
reported entries.

Reject truncated payloads before copying the report array.

Link: https://patch.msgid.link/20260517-scmi_fixes-v1-2-d86daec4defd@kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
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:** `[firmware: arm_scmi]` `[Validate]` — Validate that
`BASE_ERROR_EVENT` notification payloads are large enough for the number
of error reports claimed in `error_status`.

### Step 1.2: Commit Message Tags
**Record:**
- **Link:** `https://patch.msgid.link/20260517-scmi_fixes-v1-2-
  d86daec4defd@kernel.org`
- **Signed-off-by:** Sudeep Holla `<sudeep.holla@kernel.org>` (author;
  SCMI maintainer)
- **Reviewed-by:** Cristian Marussi `<cristian.marussi@arm.com>` (from
  mbox; SCMI co-maintainer)
- **No Fixes:, Reported-by:, Tested-by:, Cc: stable@** on this specific
  patch
- **Series context:** Patch 2/4 of `scmi_fixes-v1` (`20260517_sudeep_hol
  la_firmware_arm_scmi_fix_protocol_parsing_and_validation.mbx`)

### Step 1.3: Commit Body Analysis
**Record:**
- **Bug:** `BASE_ERROR_EVENT` has a variable-length payload;
  `error_status` encodes how many `msg_reports[]` entries follow, but
  the parser used that count without verifying the received `payld_sz`
  covered all entries.
- **Symptom:** Truncated notifications are parsed anyway; the loop
  copies `msg_reports[i]` beyond the valid received bytes.
- **Root cause:** Only an upper-bound check existed (`payld_sz <=
  sizeof(*p)`); no lower-bound check based on `cmd_count`.
- **Version info:** None in the commit message.

### Step 1.4: Hidden Bug Fix Detection
**Record:** Not disguised — this is an explicit validation/hardening fix
for out-of-bounds reads on a variable-length protocol payload.

---

## Phase 2: Diff Analysis

### Step 2.1: Change Inventory
**Record:**
- **File:** `drivers/firmware/arm_scmi/base.c` (+13 / -2 per mbox;
  user's diff is equivalent)
- **Function modified:** `scmi_base_fill_custom_report()`
- **Scope:** Single-file, surgical fix (~15 lines)

### Step 2.2: Code Flow Change
**Record:**
- **Hunk 1 (before):** After checking `payld_sz` is not larger than the
  max struct, immediately read `error_status`, derive `cmd_count`, and
  loop over `p->msg_reports[i]`.
- **Hunk 1 (after):** Compute minimum size for header fields; reject if
  `payld_sz` too small; then derive `cmd_count`; compute `expected_sz +=
  cmd_count * sizeof(msg_reports[0])`; reject truncated payloads; only
  then copy reports.
- **Path affected:** Deferred notification worker path for
  `SCMI_EVENT_BASE_ERROR_EVENT`.

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Buffer over-read / out-of-bounds access on variable-
  length payload (memory safety).
- **Mechanism:** `ERROR_CMD_COUNT(error_status)` can claim N report
  entries while `payld_sz` only contains the fixed header (8 bytes) or a
  partial array. The loop reads `p->msg_reports[i]` past the valid
  received message boundary.

### Step 2.4: Fix Quality
**Record:**
- **Quality:** Obviously correct; mirrors existing SCMI validation style
  (e.g. `scmi_system_fill_custom_report()`).
- **Regression risk:** Very low — well-formed firmware messages are
  unchanged; malformed ones are rejected (return `NULL`, event dropped
  with existing error logging in `scmi_process_event_payload()`).
- **Note:** Mbox uses `sizeof(p->agent_id) + sizeof(p->error_status)`;
  user's diff uses `offsetof(typeof(*p), msg_reports)` — functionally
  equivalent.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Buggy logic introduced in `585dfab3fb80e` ("firmware:
arm_scmi: Add base notifications support", 2020-07-01, Cristian
Marussi). Confirmed ancestor of current HEAD.

### Step 3.2: Fixes: Tag
**Record:** N/A — no `Fixes:` tag on this commit.

### Step 3.3: Related File History
**Record:**
- `3b0041f6e10e5` — "Validate BASE_DISCOVER_LIST_PROTOCOLS response"
  (same subsystem, same validation pattern; already in this tree)
- `11daac2817dca` — "Fix OOB in scmi_power_name_get()" (already
  backported to this 6.18.y tree)
- `bac3e70c2fb10` — patch 1/4 of the same series (sensor config width
  fix) is already in this tree; **patch 2/4 (this fix) is not**

### Step 3.4: Author Context
**Record:** Sudeep Holla is the SCMI subsystem maintainer. Recent SCMI
commits in this tree include multiple validation and OOB fixes.

### Step 3.5: Dependencies
**Record:** Standalone — only touches `base.c`. Does not depend on patch
1/4 (sensors), 3/4, or 4/4. Applies independently.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
**Record:** `b4 dig` could not be used (commit not in tree).
Lore/patch.msgid.link fetch blocked (403/bot protection). Used local
mbox: `20260517_sudeep_holla_firmware_arm_scmi_fix_protocol_parsing_and_
validation.mbx`. Series v1, patch 2/4.

### Step 4.2: Reviewers
**Record:** Reviewed-by Cristian Marussi on patch 2/4. Cover letter Cc's
`arm-scmi@vger.kernel.org`, `linux-arm-kernel@lists.infradead.org`.

### Step 4.3: Bug Report
**Record:** No external bug report or syzbot link. Issue found during
spec-compliance review per cover letter ("checking the driver message
layouts against the SCMI specification").

### Step 4.4: Series Context
**Record:** 4-patch series; each patch is independently valuable. Patch
1 already present in tree; patches 2–4 are separate fixes.

### Step 4.5: Stable List History
**Record:** Not searched (lore blocked). Cover letter does not
explicitly request stable, but that is not a negative signal per
instructions.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `scmi_base_fill_custom_report()` (modified); callers via
`REVT_FILL_REPORT` macro.

### Step 5.2: Callers
**Record:** Called from `scmi_process_event_payload()` in `notify.c`
(line 495), which runs in a workqueue context after `scmi_notify()`
queues firmware events from interrupt context.

### Step 5.3: Callees
**Record:** `le32_to_cpu()`, `le64_to_cpu()`, `IS_FATAL_ERROR()`,
`ERROR_CMD_COUNT()`, field access on `payld` and `report` buffers.

### Step 5.4: Reachability
**Record:**
- `scmi_notify()` ← SCMI transport RX path (firmware/platform
  notifications)
- Not directly userspace-syscall reachable, but triggered by SCMI
  platform firmware on ARM systems using SCMI
- Affects any platform where `BASE_ERROR_EVENT` notifications are
  enabled

### Step 5.5: Similar Patterns
**Record:** `scmi_system_fill_custom_report()` already validates
`payld_sz == expected_sz`. `3b0041f6e10e5` validates variable-length
protocol list responses. Same hardening pattern.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Exists?
**Record:** **Yes.** Local tree is **v6.18.44** (`git describe HEAD`).
`scmi_base_fill_custom_report()` at lines 322–350 in `base.c` lacks
`expected_sz` validation. Bug present since v5.7-era introduction
(2020).

### Step 6.2: Backport Complications
**Record:** Expected **clean apply** — current `base.c` matches the
patch context exactly. No `expected_sz` present. Mbox patch context
matches current file structure.

### Step 6.3: Related Fixes Already Present?
**Record:** Patch 1/4 (`bac3e70c2fb10`) is in tree. This specific
BASE_ERROR_EVENT validation is **not** present. No duplicate fix found.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem Criticality
**Record:** `drivers/firmware/arm_scmi/` — **IMPORTANT** subsystem for
ARM/ARM64 platforms (servers, embedded, mobile SoCs using SCMI to talk
to SCP/EL3 firmware).

### Step 7.2: Subsystem Activity
**Record:** Actively maintained; recent commits include OOB fixes, NULL
deref fixes, and validation hardening.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Platforms using SCMI with `BASE_ERROR_EVENT` notifications
enabled (`CONFIG_ARM_SCMI_PROTOCOL`). Driver-specific / platform-
specific, but SCMI is widespread on modern ARM hardware.

### Step 8.2: Trigger Conditions
**Record:** Firmware sends a `BASE_ERROR_EVENT` where `error_status`
claims more `msg_reports` than the actual payload contains. Can result
from buggy firmware, transport corruption, or malformed messages. Not
directly triggerable by unprivileged userspace, but firmware input is
treated as untrusted in hardening contexts.

### Step 8.3: Failure Mode Severity
**Record:**
- **Without fix:** Reads beyond valid received payload into the pre-
  allocated scratch buffer (`pd->eh`, sized to max payload). This can
  return **stale/uninitialized kernel data** as error reports to
  registered event handlers — information leak and incorrect error
  reporting.
- **With fix:** Returns `NULL`; event is dropped with `"report not
  available"` error (existing path).
- **Severity:** **HIGH** (out-of-bounds read / info leak pattern); crash
  is less likely because scratch buffer is pre-allocated to max size,
  but corrupted reports are a real correctness and security concern.

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** HIGH for affected ARM SCMI platforms — prevents parsing
  truncated firmware notifications and leaking stale data.
- **Risk:** VERY LOW — small, obviously correct validation; no behavior
  change for well-formed messages.
- **Ratio:** Strong benefit, minimal risk.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real memory-safety bug in variable-length notification parsing
- Long-standing (since 2020), present in v6.18.44
- Small, surgical, maintainer-reviewed fix
- Matches established SCMI validation pattern already in this tree
- Precedent: similar SCMI OOB/validation fixes already backported here
  (`11daac2817dca`, `3b0041f6e10e5`)
- Standalone — no series dependencies
- No functional change for correct firmware

**AGAINST backport:**
- Trigger requires malformed firmware notification (not common in
  production, but possible)
- Not syzbot-reported or user-reported with crash trace
- Patch 2/4 lacks the extensive `Tested-by:` list that patch 1/4 has
  (though it has `Reviewed-by`)

**Unresolved:**
- Could not access lore.kernel.org directly (403/bot protection)
- `b4 dig` not usable without commit in tree

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — logic is straightforward;
   Reviewed-by subsystem co-maintainer
2. Fixes a real bug affecting users? **PASS** — truncated payload
   parsing on real ARM SCMI hardware
3. Important issue? **PASS** — out-of-bounds read / stale data leak
   (HIGH)
4. Small and contained? **PASS** — ~15 lines, one file, one function
5. No new features or APIs? **PASS**
6. Can apply to local tree? **PASS** — buggy code confirmed present;
   patch is standalone

### Step 9.3: Exception Categories
**Record:** N/A — standard bug fix, not a device-ID/quirk/DT/build/docs
exception.

### Step 9.4: Problem Summary for Stable Users

On ARM systems using SCMI, `BASE_ERROR_EVENT` notifications report
firmware errors with a variable number of 64-bit report words. The
kernel driver trusted the count in `error_status` without verifying the
received message was large enough. A truncated notification could cause
the driver to read beyond the valid payload into scratch-buffer memory
and forward garbage/stale data to event handlers.

The fix adds minimum-size checks before parsing — the same defensive
pattern already used elsewhere in SCMI (e.g. system power-state
notifications, protocol list discovery). It is small, maintainer-
reviewed, and appropriate for the v6.18.y stable tree where the
vulnerable code is present.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body from user-provided commit and
  local mbox `20260517_sudeep_holla_firmware_arm_scmi_fix_protocol_parsi
  ng_and_validation.mbx`
- **[Phase 1]** Found Reviewed-by: Cristian Marussi in mbox patch 2/4
- **[Phase 2]** Read current `scmi_base_fill_custom_report()` at lines
  322–350 in `drivers/firmware/arm_scmi/base.c` — missing validation
- **[Phase 2]** Confirmed `SCMI_BASE_MAX_CMD_ERR_COUNT` = 1024, struct
  layout with variable reports
- **[Phase 3]** `git blame -L 322,350`: buggy code from `585dfab3fb80e`
  (2020-07-01)
- **[Phase 3]** `git merge-base --is-ancestor 585dfab3fb80e HEAD`:
  confirmed in tree
- **[Phase 3]** `git log --oneline -20 --
  drivers/firmware/arm_scmi/base.c`: related validation commit
  `3b0041f6e10e5` present
- **[Phase 3]** Confirmed `bac3e70c2fb10` (series patch 1/4) in tree;
  patch 2/4 not in tree
- **[Phase 4]** `b4 dig -c HEAD`: failed (commit not in tree)
- **[Phase 4]** WebFetch lore/patch.msgid.link: blocked (403/bot
  protection)
- **[Phase 4]** Read local mbox cover letter and patch 2/4 content
- **[Phase 5]** Traced call chain: `scmi_notify()` → workqueue →
  `scmi_process_event_payload()` → `REVT_FILL_REPORT()` →
  `scmi_base_fill_custom_report()`
- **[Phase 5]** Read `scmi_process_event_payload()` NULL-report handling
  at lines 498–502 in `notify.c`
- **[Phase 5]** Read `scmi_system_fill_custom_report()` validation
  pattern in `system.c`
- **[Phase 5]** Read scratch buffer allocation in
  `scmi_allocate_registered_events_desc()` — `eh_sz` = max payload +
  header
- **[Phase 6]** `git describe HEAD`: v6.18.44
- **[Phase 6]** `grep expected_sz drivers/firmware/arm_scmi/base.c`: no
  matches — fix not applied
- **[Phase 6]** Patch context in mbox matches current `base.c` structure
- **[Phase 7]** Confirmed SCMI is active subsystem with recent security
  fixes in this tree
- **[Phase 8]** Assessed failure mode: OOB read of stale scratch-buffer
  data, not typical kmalloc overflow
- **UNVERIFIED:** Direct lore.kernel.org thread content (blocked)
- **UNVERIFIED:** Whether this exact commit SHA exists on mainline
  (evaluated from patch content against local tree)

**YES**

 drivers/firmware/arm_scmi/base.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index 86b376c50a13f..25aa52746bd10 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -325,6 +325,8 @@ static void *scmi_base_fill_custom_report(const struct scmi_protocol_handle *ph,
 					  void *report, u32 *src_id)
 {
 	int i;
+	u32 error_status;
+	size_t expected_sz;
 	const struct scmi_base_error_notify_payld *p = payld;
 	struct scmi_base_error_report *r = report;
 
@@ -338,10 +340,19 @@ static void *scmi_base_fill_custom_report(const struct scmi_protocol_handle *ph,
 	if (evt_id != SCMI_EVENT_BASE_ERROR_EVENT || sizeof(*p) < payld_sz)
 		return NULL;
 
+	expected_sz = offsetof(typeof(*p), msg_reports);
+	if (payld_sz < expected_sz)
+		return NULL;
+
 	r->timestamp = timestamp;
 	r->agent_id = le32_to_cpu(p->agent_id);
-	r->fatal = IS_FATAL_ERROR(le32_to_cpu(p->error_status));
-	r->cmd_count = ERROR_CMD_COUNT(le32_to_cpu(p->error_status));
+	error_status = le32_to_cpu(p->error_status);
+	r->fatal = IS_FATAL_ERROR(error_status);
+	r->cmd_count = ERROR_CMD_COUNT(error_status);
+	expected_sz += r->cmd_count * sizeof(p->msg_reports[0]);
+	if (payld_sz < expected_sz)
+		return NULL;
+
 	for (i = 0; i < r->cmd_count; i++)
 		r->reports[i] = le64_to_cpu(p->msg_reports[i]);
 	*src_id = 0;
-- 
2.53.0