[PATCH AUTOSEL 6.18-5.15] firmware: arm_scmi: Validate SENSOR_UPDATE payload size
Sasha Levin
sashal at kernel.org
Mon Aug 31 06:26:26 PDT 2026
From: Sudeep Holla <sudeep.holla at 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 at arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla at kernel.org>
Signed-off-by: Sasha Levin <sashal at 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 at kernel.org
- `Reviewed-by: Cristian Marussi <cristian.marussi at arm.com>` (ARM SCMI
maintainer)
- `Signed-off-by: Sudeep Holla <sudeep.holla at kernel.org>` (SCMI
maintainer)
- No `Fixes:`, `Reported-by:`, `Cc: stable at 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 at 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 at vger.kernel.org`,
`linux-arm-kernel at 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 at 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
More information about the linux-arm-kernel
mailing list