From: Joonwon Kang <joonwonkang@google.com>

[ Upstream commit 96a3d2f3167f5644b30e60171898e67123c3c2c6 ]

When the mailbox controller failed transmitting message, the error code
was only passed to the client's tx done handler and not to
mbox_send_message() in blocking mode. For this reason, the function could
return a false success. This commit resolves the issue by introducing the
tx status and checking it before mbox_send_message() returns.

This commit works with the premise that the multi-threads' access to a
channel in blocking mode is serialized by clients, not by the mailbox
APIs, since the current mbox_send_message() in blocking mode does not
support multi-threads.

Signed-off-by: Joonwon Kang <joonwonkang@google.com>
Reviewed-by: Sudeep Holla <sudeep.holla@kernel.org>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
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:** `[mailbox]` `[Make]` — Make `mbox_send_message()` return an
error code when TX fails in blocking mode.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Joonwon Kang `<joonwonkang@google.com>` (author)
- **Reviewed-by:** Sudeep Holla `<sudeep.holla@kernel.org>` (mailbox/ARM
  maintainer)
- **Signed-off-by:** Jassi Brar `<jassisinghbrar@gmail.com>` (mailbox
  subsystem maintainer)
- **No** `Fixes:`, `Reported-by:`, `Tested-by:`, `Link:`, or `Cc:
  stable` in the committed message (v6 submission notes Cc was removed
  from the commit message but was present in the patch email)
- Notable: reviewed by subsystem maintainer; patch series explicitly
  CC'd `stable@vger.kernel.org` in v6 submission

### Step 1.3: Body analysis
**Record:**
- **Bug:** In blocking mode (`tx_block`), when the mailbox controller
  reports a TX failure via `mbox_chan_txdone()` / `tx_tick()`, the error
  is delivered only to the optional `tx_done` callback.
  `mbox_send_message()` still returns the positive queue index from
  `add_to_rbuf()`, i.e. false success.
- **Symptom:** Callers checking `ret < 0` believe TX succeeded; they may
  proceed or wait for RX that never arrives, eventually timing out with
  the wrong error.
- **Root cause:** `mbox_send_message()` only converts timeout (`ret ==
  0` from `wait_for_completion_timeout`) to `-ETIME`; it never inspects
  the TX result passed to `tx_tick()`.
- **Constraint:** Author documents that blocking mode does not support
  multi-threaded concurrent senders; clients must serialize access.

### Step 1.4: Hidden bug fix?
**Record:** Yes — this is a straightforward API correctness bug fix, not
cosmetic cleanup. The kerneldoc for `mbox_send_message()` states
blocking mode should return after transmission completes; TX failure is
not success.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- `drivers/mailbox/mailbox.c`: +5 / -1 lines
- `include/linux/mailbox_controller.h`: +2 lines (field + comment)
- **Functions modified:** `tx_tick()`, `mbox_send_message()`
- **Scope:** Single-subsystem, surgical fix (7 lines net)

### Step 2.2: Code flow changes
**Record:**
- **Hunk 1 (`tx_tick`):** Before: on non-timeout completion in blocking
  mode, only called `complete()`. After: also stores `chan->tx_status =
  r` before waking the waiter.
- **Hunk 2 (`mbox_send_message`):** Before: after successful wait,
  always returned positive queue index `t`. After: if `chan->tx_status <
  0`, returns that error instead.
- **Affected path:** Blocking-mode TX completion and error reporting
  only.

### Step 2.3: Bug mechanism
**Record:**
- **Category:** Logic / API contract fix (incorrect return value
  propagation)
- **Mechanism:** TX error `r` flows through `tx_tick(chan, r)` →
  `tx_done` callback, but `mbox_send_message()` waiter was not told. Fix
  stores `r` in per-channel `tx_status` and propagates it to the return
  value.

### Step 2.4: Fix quality
**Record:**
- Fix is minimal and obviously correct for single-threaded blocking use
  (the documented/accepted model).
- **Regression risk:** Low for intended use. Reviewer Sudeep Holla noted
  per-channel `tx_status` can be stale/overwritten with concurrent
  blocking senders; author and maintainers accepted this as a pre-
  existing limitation (blocking mode is not multi-thread safe).
- No public API change; only corrects return semantics.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Current `tx_tick()` / `mbox_send_message()` blocking path in
this tree comes from commit `a112b91dd6349` (Jeff Layton, 2026-07-23) —
a sunrpc backport that brought in the current `mailbox.c` content. The
buggy blocking-return logic is present in that version. This tree's git
history is shallow for `drivers/mailbox/` (only one commit shown), so
the original introduction date of `tx_block` cannot be determined from
this checkout alone.

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

### Step 3.3: Related file history
**Record:** `git log --oneline -20 -- drivers/mailbox/mailbox.c` shows
only `a112b91dd6349`. No related fix or prerequisite series is visible
in this tree. The final v6 patch is standalone (detached from earlier
multi-thread completion work per submission changelog).

### Step 3.4: Author context
**Record:** Joonwon Kang (Google). Jassi Brar is the mailbox maintainer
and committed/acked. Sudeep Holla reviewed. No other commits from these
authors appear in this tree's mailbox history.

### Step 3.5: Dependencies
**Record:** No dependencies on other patches. Diff matches current tree
structure (`scoped_guard`, same function layout). Applies cleanly to
this tree's `mailbox.c` and `mailbox_controller.h`.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original discussion
**Record:**
- **URL:** https://lists.openwall.net/linux-kernel/2026/05/10/136
  (`[PATCH v6] mailbox: Make mbox_send_message() return error code when
  tx fails`)
- Series evolved v1→v6; v4 detached from multi-thread completion patch;
  v6 is the final standalone version matching the analyzed diff.
- Reviewer concern (Sudeep Holla, v4 thread): per-channel
  `tx_status`/`tx_complete` unsafe with concurrent blocking senders.
  Author response: blocking mode does not support multi-thread; clients
  serialize.

### Step 4.2: Reviewers
**Record:** To: Jassi Brar, Sudeep Holla. Cc: `linux-kernel`,
`stable@vger.kernel.org`, `akpm`, `dianders`. Subsystem maintainers were
directly involved.

### Step 4.3: Bug reports
**Record:** No syzbot, Bugzilla, or user crash reports. Bug identified
through API behavior analysis during a blocking-mode improvement series.

### Step 4.4: Related patches
**Record:** Earlier series `[PATCH v3 0/2]` included a per-thread
completion patch (not in this final commit). This commit is self-
contained.

### Step 4.5: Stable list
**Record:** Could not search lore.kernel.org/stable (bot protection).
Patch submission explicitly CC'd `stable@vger.kernel.org`.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key functions
**Record:** `tx_tick()`, `mbox_send_message()`, indirectly
`mbox_chan_txdone()`, `mbox_client_txdone()`, `msg_submit()`.

### Step 5.2: Callers
**Record:** `mbox_send_message()` is used across ~40+ in-tree files
(firmware, remoteproc, ACPI PCC, RPMSG, media, crypto, etc.). Blocking-
mode users (`tx_block = true`) include at minimum:
- `drivers/firmware/raspberrypi.c`
- `drivers/firmware/arm_scpi.c`
- `drivers/remoteproc/imx_dsp_rproc.c`
- `drivers/remoteproc/stm32_rproc.c`
- `drivers/soc/microchip/mpfs-sys-controller.c`
- `drivers/firmware/thead,th1520-aon.c`
- `drivers/i2c/busses/i2c-xgene-slimpro.c`
- others

Many check `ret < 0` after `mbox_send_message()`.

### Step 5.3: Callees
**Record:** `add_to_rbuf()`, `msg_submit()`,
`wait_for_completion_timeout()`, `complete()`, optional `tx_done`
callback, controller `send_data()` / IRQ completion paths.

### Step 5.4: Reachability
**Record:** Reachable from normal driver probe/runtime on SoCs using
mailbox firmware interfaces (RPi, ARM SCPI, i.MX DSP remoteproc,
Microchip MPFS, etc.). Not a syscall path, but common on embedded/ARM
platforms.

### Step 5.5: Similar patterns
**Record:** Timeout path already returns `-ETIME` correctly. Only non-
timeout TX errors were mishandled. No other instances of this exact bug
pattern elsewhere in the mailbox core.

---

## Phase 6: Cross-Reference Against Local Tree (6.18.43)

### Step 6.1: Buggy code present?
**Record:** **Yes.** Local tree is `v6.18.43-1-gc7f0dac02d232`
(Makefile: 6.18.43). `drivers/mailbox/mailbox.c` lines 103–104 and
269–276 show the buggy behavior (no `tx_status`, no error propagation).
`include/linux/mailbox_controller.h` has no `tx_status` field.

### Step 6.2: Backport complications
**Record:** **Clean apply expected.** Current tree matches the patch
context (`scoped_guard` in `tx_tick`, same line structure). Only 2 files
touched.

### Step 6.3: Fix already present?
**Record:** **No.** `git grep tx_status` in mailbox code returns
nothing. Fix is not in this tree.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem criticality
**Record:** `drivers/mailbox/` — **IMPORTANT** infrastructure for
firmware/SoC communication (SCPI, RPi firmware, remoteproc kick/stop,
system controllers).

### Step 7.2: Activity
**Record:** Limited history visible in this stable checkout; mailbox
core is mature infrastructure with long-standing blocking-mode API.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who is affected
**Record:** Platforms using mailbox clients with `tx_block = true` —
embedded ARM, RPi, i.MX, STM32 remoteproc, Microchip MPFS, ACPI PCC
paths, etc. Config-dependent (`CONFIG_MAILBOX` and specific drivers).

### Step 8.2: Trigger conditions
**Record:** Triggered when a mailbox controller reports TX failure (via
`mbox_chan_txdone(chan, negative_error)`) while client uses blocking
mode. Requires actual controller TX failure (hardware/firmware error
path). Not user-triggerable from syscalls directly, but common in
firmware interaction error paths.

### Step 8.3: Failure mode severity
**Record:**
- **Failure mode:** False success return; callers miss immediate TX
  error; may wait for RX/timeouts with wrong error code; degraded
  firmware/remoteproc operation.
- **Severity:** **MEDIUM** — not a direct crash/UAF/security issue, but
  causes incorrect error handling and can lead to multi-second hangs
  (e.g. SCPI `MAX_RX_TIMEOUT`) instead of immediate failure.

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** MEDIUM — restores documented blocking-mode semantics for
  widely used firmware paths; enables proper error handling in drivers
  that check return values.
- **Risk:** LOW — 7-line change, maintainer-reviewed, no behavior change
  for successful TX or for non-blocking mode.
- **Ratio:** Benefit outweighs risk for this tree.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence summary

**FOR backport:**
- Real, reproducible API bug: false success on TX failure in blocking
  mode
- Buggy code confirmed present in Linux 6.18.43 checkout
- Affects multiple production drivers using `tx_block` + `ret < 0`
  checks
- Small, surgical, maintainer-reviewed fix
- Patch series CC'd stable; standalone, no dependencies
- Corrects behavior to match kerneldoc contract

**AGAINST backport:**
- No crash, UAF, security, or data-corruption report
- Severity is incorrect error propagation / delayed failure, not system
  panic
- Pre-existing multi-thread limitation in blocking mode (reviewer
  concern)
- No fuzzer or user bug report

**Unresolved:** Exact kernel version when `tx_block` was introduced
(shallow history in this tree).

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — logic is clear; Reviewed-by
   maintainer; no Tested-by but straightforward.
2. Fixes a real bug affecting users? **PASS** — false success on TX
   failure in blocking mode.
3. Important issue? **PASS (borderline)** — MEDIUM severity API bug on
   firmware-critical paths; wrong error handling and potential hangs.
4. Small and contained? **PASS** — 7 lines, 2 files.
5. No new features or APIs? **PASS** — internal `tx_status` field;
   corrects existing return semantics.
6. Can apply to local tree? **PASS** — buggy code present; clean apply
   expected.

### Step 9.3: Exception categories
**Record:** None (not a device ID, quirk, DT, build fix, or docs-only
change).

### Step 9.4: Decision rationale
For **this** 6.18.y tree, the buggy blocking-mode behavior exists today
and affects real firmware/remoteproc drivers that rely on
`mbox_send_message()` return values. The fix is minimal, low-risk, and
restores the documented API contract. While not a crash-level bug,
incorrect error propagation on mailbox TX failures is meaningful for
stable users on embedded platforms, and the patch was explicitly
prepared for stable with maintainer review.

---

## Verification

- **[Phase 1]** Parsed commit message and tags from user-provided diff
  and lore submission
- **[Phase 2]** Read `drivers/mailbox/mailbox.c` lines 84–105, 245–277;
  confirmed buggy logic (no `tx_status`, returns queue index on TX
  error)
- **[Phase 2]** Read `include/linux/mailbox_controller.h`; confirmed no
  `tx_status` field
- **[Phase 3]** `git describe HEAD` → `v6.18.43-1-gc7f0dac02d232`;
  Makefile → 6.18.43
- **[Phase 3]** `git blame -L 84,105 drivers/mailbox/mailbox.c` → all
  from `a112b91dd6349`
- **[Phase 3]** `git log --oneline -20 -- drivers/mailbox/mailbox.c` →
  only one commit in this tree
- **[Phase 3]** `git log --grep` for commit subject → not found in tree
  (fix not merged)
- **[Phase 4]** Fetched https://lists.openwall.net/linux-
  kernel/2026/05/10/136 — confirmed v6 patch, Reviewed-by Sudeep Holla,
  Cc stable in submission
- **[Phase 4]** Web search found Sudeep Holla multi-thread review
  concern and author response
- **[Phase 4]** `b4 dig -c` not run — commit not in this tree; `b4 dig
  -q` unsupported
- **[Phase 4]** lore.kernel.org/stable search blocked by bot protection
- **[Phase 5]** `grep mbox_send_message(` — 40+ call sites across
  firmware/remoteproc/ACPI/etc.
- **[Phase 5]** `grep tx_block = true` — 10+ blocking-mode clients
  identified
- **[Phase 5]** Read `raspberrypi.c:49-71`, `arm_scpi.c:512-520`, `mpfs-
  sys-controller.c:50-74`, `imx_dsp_rproc.c:611` — all use blocking mode
  and check return values
- **[Phase 6]** Confirmed fix absent: no `tx_status` in mailbox code
- **[Phase 6]** Confirmed patch context matches current tree
  (`scoped_guard` present in `tx_tick`)
- **[Phase 7]** Mailbox is firmware/SoC communication infrastructure —
  IMPORTANT
- **[Phase 8]** Failure mode: false success → wrong error handling / RX
  wait timeouts — MEDIUM severity
- **UNVERIFIED:** Exact mainline commit SHA (not in this tree); date
  `tx_block` was originally introduced

**YES**

 drivers/mailbox/mailbox.c          | 6 +++++-
 include/linux/mailbox_controller.h | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 60866e8417c9b..0f2019aadc216 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -100,8 +100,10 @@ static void tx_tick(struct mbox_chan *chan, int r)
 	if (chan->cl->tx_done)
 		chan->cl->tx_done(chan->cl, mssg, r);
 
-	if (r != -ETIME && chan->cl->tx_block)
+	if (r != -ETIME && chan->cl->tx_block) {
+		chan->tx_status = r;
 		complete(&chan->tx_complete);
+	}
 }
 
 static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
@@ -270,6 +272,8 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
 		if (ret == 0) {
 			t = -ETIME;
 			tx_tick(chan, t);
+		} else if (chan->tx_status < 0) {
+			t = chan->tx_status;
 		}
 	}
 
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index 1db0069c27c52..e6af0721198b4 100644
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -114,6 +114,7 @@ struct mbox_controller {
  * @txdone_method:	Way to detect TXDone chosen by the API
  * @cl:			Pointer to the current owner of this channel
  * @tx_complete:	Transmission completion
+ * @tx_status:		Transmission status
  * @active_req:		Currently active request hook
  * @msg_count:		No. of mssg currently queued
  * @msg_free:		Index of next available mssg slot
@@ -126,6 +127,7 @@ struct mbox_chan {
 	unsigned txdone_method;
 	struct mbox_client *cl;
 	struct completion tx_complete;
+	int tx_status;
 	void *active_req;
 	unsigned msg_count, msg_free;
 	void *msg_data[MBOX_TX_QUEUE_LEN];
-- 
2.53.0