[PATCH AUTOSEL 6.18] ASoC: SOF: validate probe info element counts

[Sasha Levin <sashal@kernel.org>]

From: Yousef Alhouseen <alhouseenyousef@gmail.com>

[ Upstream commit 95edf2dbb492f3ea2420111e9c0044c7dec9113c ]

Probe information replies contain a firmware-provided element count. IPC3
uses that count to copy an array, then returns the unchecked count to its
caller. A short reply can therefore make the caller walk beyond the copied
array.

IPC4 similarly uses the count both to allocate the destination array and
to walk the reply. On 32-bit systems the allocation size can wrap, while on
all systems an excessive count reads beyond the reply payload.

Validate each count against the actual reply size before copying or
allocating the array, and use kcalloc() for the IPC4 allocation.

Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
Link: https://patch.msgid.link/20260628000329.18606-1-alhouseenyousef@gmail.com
Signed-off-by: Mark Brown <broonie@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:** `[ASoC: SOF]` `[validate]` — Validate firmware-provided
probe info element counts before use in IPC3/IPC4 probe handlers.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Yousef Alhouseen `<alhouseenyousef@gmail.com>`
  (author)
- **Link:** `https://patch.msgid.link/20260628000329.18606-1-
  alhouseenyousef@gmail.com`
- **Signed-off-by:** Mark Brown `<broonie@kernel.org>` (ASoC maintainer)
- No `Fixes:`, `Reported-by:`, `Cc: stable@vger.kernel.org`, `Tested-
  by:`, or `Reviewed-by:` tags
- Notable: maintainer sign-off; no fuzzer/user bug report in message

### Step 1.3: Body Analysis
**Record:**
- **Bug:** Probe info IPC replies include a firmware-provided
  `num_elems`. IPC3 uses it for `kmemdup()` and returns it to callers;
  IPC4 uses it for allocation and iteration. Neither path validates the
  count against the actual reply size.
- **Symptoms:** Short/malformed replies can cause out-of-bounds reads
  during copy/iteration; on 32-bit IPC4 paths, `num_elems *
  sizeof(desc)` can wrap in `kzalloc()`.
- **Root cause:** Missing bounds check on untrusted firmware IPC payload
  fields.
- **Version info:** None stated in the commit message.

### Step 1.4: Hidden Bug Fix?
**Record:** Yes. Although the subject says "validate" rather than "fix",
this is a memory-safety bug fix: unchecked firmware metadata can cause
OOB access and allocation-size wrap.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- `sound/soc/sof/sof-client-probes-ipc3.c`: +19 / -4 lines
- `sound/soc/sof/sof-client-probes-ipc4.c`: +11 / -1 lines
- **Functions modified:** `ipc3_probes_info()`,
  `ipc4_probes_points_info()`
- **Scope:** Small, two-file, subsystem-local surgical fix

### Step 2.2: Code Flow Changes

**IPC3 (`ipc3_probes_info`):**
- **Before:** After IPC success, used `reply->num_elems` directly to
  compute `bytes *= num_elems`, `kmemdup()`, and `*num_params`.
- **After:** Reads `payload_size = reply->rhdr.hdr.size`, rejects
  undersized payloads, computes `elem_size`, validates `num_elems <=
  payload_size / elem_size`, then copies/returns count.

**IPC4 (`ipc4_probes_points_info`):**
- **Before:** Used `info->num_elems` directly for `kzalloc(*num_desc *
  sizeof(**desc))` and loop bound.
- **After:** Validates `info->num_elems` against `msg.data_size`,
  switches to `kcalloc()`, rejects invalid counts.

### Step 2.3: Bug Mechanism
**Record:** **Memory safety / bounds validation bug**
- **IPC3:** Unchecked `num_elems` can make `bytes = elem_size *
  num_elems` exceed actual reply payload; `kmemdup()` reads past valid
  IPC data. If multiplication wraps, a small allocation can be paired
  with a large returned count, and callers iterate past the allocation.
- **IPC4:** Unchecked `num_elems` allows loop reads past `msg.data_ptr`
  bounds; `kzalloc(n * size)` can wrap on 32-bit systems.

### Step 2.4: Fix Quality
**Record:** Fix is obviously correct and minimal. It mirrors the
existing SOF pattern in `debug.c` (`struct_size(reply, elems,
reply->num_elems) != reply->rhdr.hdr.size`). Regression risk is very
low: only rejects malformed firmware replies.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Buggy lines in both files trace to commit `5d324e5159d9e` in
this shallow checkout. The vulnerable logic is present in the current
tree at `sof-client-probes-ipc3.c:131-144` and `sof-client-probes-
ipc4.c:251-264`.

### Step 3.2: Fixes Tag
**Record:** N/A — no `Fixes:` tag in the commit message.

### Step 3.3: Related File History
**Record:** Repository is shallow (`git rev-parse --is-shallow-
repository` → `true`), limiting history depth. The probe client files
exist in this 6.18.44 tree. No duplicate fix found (`grep "invalid probe
info element count"` → no matches).

### Step 3.4: Author History
**Record:** No prior SOF commits from Yousef Alhouseen found in this
tree. Mark Brown is ASoC maintainer (sign-off).

### Step 3.5: Dependencies
**Record:** Standalone fix. Uses only existing headers (`offsetof`,
`kcalloc`). No series dependency indicated. The commit is not yet
present in this checkout.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
**Record:** UNVERIFIED — `b4 dig` could not match the commit (not in
local repo). WebFetch to patch.msgid.link and lore.kernel.org returned
bot-protection pages (403/JS challenge). Could not read review thread.

### Step 4.2: Reviewers
**Record:** UNVERIFIED — `b4 dig -w` unavailable for this commit.

### Step 4.3: Bug Report
**Record:** No `Reported-by:` or syzbot link. Issue identified by code
inspection of firmware IPC parsing.

### Step 4.4: Related Patches
**Record:** UNVERIFIED — could not retrieve series revisions from lore.

### Step 4.5: Stable List History
**Record:** UNVERIFIED — lore stable search inaccessible.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `ipc3_probes_info()`, `ipc3_probes_points_info()` (wrapper),
`ipc4_probes_points_info()`

### Step 5.2: Callers
**Record:**
- `sof_probes_compr_shutdown()` in `sof-client-probes.c:78` —
  compressed-stream shutdown path
- `sof_probes_dfs_points_read()` in `sof-client-probes.c:227` — debugfs
  read path (root-accessible)

Both invoke `ipc->points_info()` from the IPC ops table.

### Step 5.3: Callees
**Record:** `sof_client_ipc_tx_message()`,
`sof_client_ipc_set_get_data()`, `kmemdup()`, `kzalloc()`/`kcalloc()`,
`sof_client_get_ipc_max_payload_size()`

### Step 5.4: Reachability
**Record:**
- Trigger requires `CONFIG_SND_SOC_SOF_DEBUG_PROBES`, auto-selected on
  Intel HDA (`SND_SOC_SOF_HDA_PROBES`) and AMD ACP
  (`SND_SOC_SOF_ACP_PROBES`) SOF platforms.
- Malformed `num_elems` must come from SOF firmware IPC replies during
  probe point enumeration.
- Not a direct unprivileged syscall path, but reachable during normal
  audio probe shutdown and root debugfs use when probes are active.
- Precedent: `sound/soc/sof/debug.c:227-231` already validates similar
  IPC `num_elems` against `rhdr.hdr.size`.

### Step 5.5: Similar Patterns
**Record:** `debug.c` already validates IPC element counts; probes code
was missing equivalent checks. `ipc3-control.c` uses overflow checks for
control data sizes.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
**Record:** **Yes.** Local tree is **v6.18.44** (`6.18.44`). Vulnerable
code is present; fix is **not** applied. Confirmed by reading current
sources and absent error string `invalid probe info element count`.

### Step 6.2: Backport Complications
**Record:** Expected **clean apply** — current file contents match the
patch base context exactly.

### Step 6.3: Related Fixes Already Present?
**Record:** No equivalent validation found in probe IPC files. `debug.c`
has similar validation for a different IPC path only.

---

## Phase 7: Subsystem and Maintainer Context

### Step 7.1: Subsystem
**Record:** `sound/soc/sof` — ASoC / SOF audio driver. **Criticality:
IMPORTANT** (not core kernel, but widely used on Intel/AMD
laptop/desktop SOF platforms).

### Step 7.2: Activity
**Record:** SOF client probe support is active in this tree (`sof-
client-probes*.c` present, Makefile builds with
`CONFIG_SND_SOC_SOF_DEBUG_PROBES`).

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users on SOF platforms with probes enabled (Intel HDA SOF,
AMD ACP). Config-specific, not universal.

### Step 8.2: Trigger Conditions
**Record:** SOF firmware returns probe info with `num_elems`
inconsistent with reply size. Requires probes feature active and a
probe-info IPC exchange. Most likely with buggy firmware; defense-in-
depth against compromised firmware is also relevant. Root can trigger
via debugfs when extractor is running.

### Step 8.3: Failure Mode Severity
**Record:**
- **IPC3:** OOB read in `kmemdup()`; potential `size_t` multiply wrap
  leading to small allocation + large iteration count
- **IPC4:** OOB read in `info->points[i]` loop; `kzalloc()` size wrap on
  32-bit
- **Severity: HIGH** (kernel memory safety; possible oops/KASAN fault)

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** Prevents kernel memory corruption/crash on malformed
  firmware IPC in an existing code path
- **Risk:** Very low — only rejects invalid replies; follows established
  SOF validation pattern
- **Ratio:** Strong benefit, minimal risk

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real memory-safety bug (OOB read, allocation-size wrap)
- Small, surgical, obviously correct fix
- Buggy code exists in this 6.18.44 tree; fix not yet present
- Matches existing validation in `debug.c`
- Maintainer-reviewed (Mark Brown)
- Callers iterate using returned count after undersized allocation/copy

**AGAINST backport:**
- Config-limited (`CONFIG_SND_SOC_SOF_DEBUG_PROBES`)
- Trigger requires malformed SOF firmware IPC, not direct userspace
  input
- No syzbot/user report in commit message
- Mailing list review details unavailable

**Unresolved:**
- Full lore review thread and any explicit stable nominations (web
  access blocked)

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — logic is straightforward;
   maintainer sign-off; pattern proven elsewhere in SOF
2. Fixes a real bug affecting users? **PASS** — malformed firmware IPC
   can crash/affect SOF probe users
3. Important issue? **PASS** — HIGH severity memory safety
4. Small and contained? **PASS** — ~30 lines, 2 files
5. No new features or APIs? **PASS** — validation only
6. Can apply to local tree? **PASS** — code exists; patch context
   matches

### Step 9.3: Exception Categories
**Record:** None (not device ID, quirk, DT, build, or docs). Standard
bug-fix exception applies.

### Step 9.4: Decision Rationale

For **this** tree (6.18.44), the vulnerable probe IPC parsing code is
present and lacks bounds checking on firmware-provided element counts.
The fix is minimal, follows an established SOF pattern, and closes a
real kernel memory-safety hole that can be triggered when SOF probes are
in use and firmware returns inconsistent IPC data. While the feature is
config-specific and firmware-mediated, stable trees routinely take such
IPC validation fixes because the failure mode is kernel OOB access, not
a benign error return.

---

## Verification

- [Phase 1] Parsed subject, tags, body from provided commit message
- [Phase 1] Confirmed no `Fixes:`, `Reported-by:`, or syzbot references
- [Phase 2] Read diff hunks for `ipc3_probes_info()` and
  `ipc4_probes_points_info()`
- [Phase 2] Classified bug as unchecked firmware `num_elems` → OOB /
  overflow
- [Phase 3] `git describe HEAD` → `v6.18.44-1-g2736c32da98b9`; Makefile
  → 6.18.44
- [Phase 3] `git blame` on vulnerable lines → `5d324e5159d9e`
- [Phase 3] `git rev-parse --is-shallow-repository` → `true`
- [Phase 3] `grep "invalid probe info element count"` → no matches (fix
  absent)
- [Phase 4] `b4 dig -c HEAD` on wrong commit; target commit not in repo
- [Phase 4] WebFetch patch.msgid.link and lore search → blocked by bot
  protection
- [Phase 4] curl lore raw → 403 Forbidden
- [Phase 5] `grep` callers → `sof-client-probes.c:78`, `sof-client-
  probes.c:227`
- [Phase 5] Read `debug.c:227-231` — existing `num_elems` vs `hdr.size`
  validation
- [Phase 5] Read Kconfig — `SND_SOC_SOF_DEBUG_PROBES` auto-selected by
  HDA/ACP probe options
- [Phase 6] Read current `sof-client-probes-ipc3.c:102-148` — vulnerable
  code confirmed
- [Phase 6] Read current `sof-client-probes-ipc4.c:207-267` — vulnerable
  code confirmed
- [Phase 6] Verified patch context matches current tree contents
- [Phase 7] Read `sound/soc/sof/Makefile` — probes built under
  `CONFIG_SND_SOC_SOF_DEBUG_PROBES`
- [Phase 8] Traced failure modes: OOB read, size wrap, caller over-
  iteration
- **UNVERIFIED:** Lore review thread, stable-list discussion, explicit
  reviewer stable nomination

**YES**

 sound/soc/sof/sof-client-probes-ipc3.c | 23 +++++++++++++++++++----
 sound/soc/sof/sof-client-probes-ipc4.c | 11 ++++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/sound/soc/sof/sof-client-probes-ipc3.c b/sound/soc/sof/sof-client-probes-ipc3.c
index a78ec0954a618..a3e382d6161f1 100644
--- a/sound/soc/sof/sof-client-probes-ipc3.c
+++ b/sound/soc/sof/sof-client-probes-ipc3.c
@@ -107,7 +107,7 @@ static int ipc3_probes_info(struct sof_client_dev *cdev, unsigned int cmd,
 	struct device *dev = &cdev->auxdev.dev;
 	struct sof_ipc_probe_info_params msg = {{{0}}};
 	struct sof_ipc_probe_info_params *reply;
-	size_t bytes;
+	size_t bytes, elem_size, payload_size;
 	int ret;
 
 	*params = NULL;
@@ -128,14 +128,29 @@ static int ipc3_probes_info(struct sof_client_dev *cdev, unsigned int cmd,
 	if (ret < 0 || reply->rhdr.error < 0)
 		goto exit;
 
+	payload_size = reply->rhdr.hdr.size;
+	if (payload_size < offsetof(struct sof_ipc_probe_info_params, dma)) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
 	if (!reply->num_elems)
 		goto exit;
 
 	if (cmd == SOF_IPC_PROBE_DMA_INFO)
-		bytes = sizeof(reply->dma[0]);
+		elem_size = sizeof(reply->dma[0]);
 	else
-		bytes = sizeof(reply->desc[0]);
-	bytes *= reply->num_elems;
+		elem_size = sizeof(reply->desc[0]);
+
+	payload_size -= offsetof(struct sof_ipc_probe_info_params, dma);
+	if (reply->num_elems > payload_size / elem_size) {
+		dev_err(dev, "%s: invalid probe info element count %u\n",
+			__func__, reply->num_elems);
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	bytes = reply->num_elems * elem_size;
 	*params = kmemdup(&reply->dma[0], bytes, GFP_KERNEL);
 	if (!*params) {
 		ret = -ENOMEM;
diff --git a/sound/soc/sof/sof-client-probes-ipc4.c b/sound/soc/sof/sof-client-probes-ipc4.c
index 758a56d271d77..2c2ab85979b83 100644
--- a/sound/soc/sof/sof-client-probes-ipc4.c
+++ b/sound/soc/sof/sof-client-probes-ipc4.c
@@ -248,10 +248,19 @@ static int ipc4_probes_points_info(struct sof_client_dev *cdev,
 		return ret;
 	}
 	info = msg.data_ptr;
+	if (msg.data_size < sizeof(*info) ||
+	    info->num_elems > (msg.data_size - sizeof(*info)) /
+			      sizeof(info->points[0])) {
+		dev_err(dev, "%s: invalid probe info element count %u\n",
+			__func__, info->num_elems);
+		kfree(msg.data_ptr);
+		return -EINVAL;
+	}
+
 	*num_desc = info->num_elems;
 	dev_dbg(dev, "%s: got %zu probe points", __func__, *num_desc);
 
-	*desc = kzalloc(*num_desc * sizeof(**desc), GFP_KERNEL);
+	*desc = kcalloc(*num_desc, sizeof(**desc), GFP_KERNEL);
 	if (!*desc) {
 		kfree(msg.data_ptr);
 		return -ENOMEM;
-- 
2.53.0