From: Joonwon Kang <joonwonkang@google.com>
To: stable@vger.kernel.org, jassisinghbrar@gmail.com
Cc: sudeep.holla@arm.com, thierry.reding@gmail.com,
jonathanh@nvidia.com, linux-kernel@vger.kernel.org,
linux-acpi@vger.kernel.org, linux-tegra@vger.kernel.org,
joonwonkang@google.com, Douglas Anderson <dianders@chromium.org>
Subject: [PATCH 6.18.y] mailbox: Fix NULL message support in mbox_send_message()
Date: Thu, 7 May 2026 06:21:07 +0000 [thread overview]
Message-ID: <20260507062107.2927600-1-joonwonkang@google.com> (raw)
From: Jassi Brar <jassisinghbrar@gmail.com>
commit c58e9456e30c ("mailbox: Fix NULL message support in mbox_send_message()") upstream.
The active_req field serves double duty as both the "is a TX in
flight" flag (NULL means idle) and the storage for the in-flight
message pointer. When a client sends NULL via mbox_send_message(),
active_req is set to NULL, which the framework misinterprets as
"no active request". This breaks the TX state machine by:
- tx_tick() short-circuits on (!mssg), skipping the tx_done
callback and the tx_complete completion
- txdone_hrtimer() skips the channel entirely since active_req
is NULL, so poll-based TX-done detection never fires.
Fix this by introducing a MBOX_NO_MSG sentinel value that means
"no active request," freeing NULL to be valid message data. The
sentinel is defined in the subsystem-internal mailbox.h so that
controller drivers within drivers/mailbox/ can reference it, but
it is not exposed to clients outside the subsystem.
Fifteen in-tree callers send NULL (doorbell-style IPCs on Qualcomm,
Tegra, TI, Xilinx, i.MX, SCMI, and PCC platforms). All were
audited for regression:
- Most already work around the bug via knows_txdone=true with a
manual mbox_client_txdone() call, making the framework's
tracking irrelevant. These are unaffected.
- Poll-based callers (Xilinx zynqmp/r5) are strictly better off:
the poll timer now correctly detects NULL-active channels
instead of silently skipping them.
- irq-qcom-mpm.c was a pre-existing bug -- the only Qualcomm
caller that omitted the knows_txdone + mbox_client_txdone()
pattern. Fixed in a companion commit ("irqchip/qcom-mpm: Fix
missing mailbox TX done acknowledgment").
- No caller sets both a tx_done callback and sends NULL, nor
combines tx_block=true with NULL sends, so the newly reachable
callback/completion paths are never exercised.
Also update tegra-hsp's flush callback, which directly inspects
active_req to wait for the channel to drain: the old "!= NULL"
check becomes "!= MBOX_NO_MSG", otherwise flush spins until
timeout since the sentinel is non-NULL.
The only tradeoff is that 'MBOX_NO_MSG' can not be used as a message
by clients.
Reported-by: Joonwon Kang <joonwonkang@google.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
[ add the MBOX_NO_MSG check to drivers/mailbox/pcc.c. ]
Signed-off-by: Joonwon Kang <joonwonkang@google.com>
---
drivers/mailbox/mailbox.c | 15 ++++++++-------
drivers/mailbox/pcc.c | 2 +-
drivers/mailbox/tegra-hsp.c | 2 +-
include/linux/mailbox_controller.h | 3 +++
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 2acc6ec229a4..caa98e38ce04 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -52,7 +52,7 @@ static void msg_submit(struct mbox_chan *chan)
int err = -EBUSY;
scoped_guard(spinlock_irqsave, &chan->lock) {
- if (!chan->msg_count || chan->active_req)
+ if (!chan->msg_count || chan->active_req != MBOX_NO_MSG)
break;
count = chan->msg_count;
@@ -87,13 +87,13 @@ static void tx_tick(struct mbox_chan *chan, int r)
scoped_guard(spinlock_irqsave, &chan->lock) {
mssg = chan->active_req;
- chan->active_req = NULL;
+ chan->active_req = MBOX_NO_MSG;
}
/* Submit next message */
msg_submit(chan);
- if (!mssg)
+ if (mssg == MBOX_NO_MSG)
return;
/* Notify the client */
@@ -114,7 +114,7 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
for (i = 0; i < mbox->num_chans; i++) {
struct mbox_chan *chan = &mbox->chans[i];
- if (chan->active_req && chan->cl) {
+ if (chan->active_req != MBOX_NO_MSG && chan->cl) {
txdone = chan->mbox->ops->last_tx_done(chan);
if (txdone)
tx_tick(chan, 0);
@@ -246,7 +246,7 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
{
int t;
- if (!chan || !chan->cl)
+ if (!chan || !chan->cl || mssg == MBOX_NO_MSG)
return -EINVAL;
t = add_to_rbuf(chan, mssg);
@@ -319,7 +319,7 @@ static int __mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl)
scoped_guard(spinlock_irqsave, &chan->lock) {
chan->msg_free = 0;
chan->msg_count = 0;
- chan->active_req = NULL;
+ chan->active_req = MBOX_NO_MSG;
chan->cl = cl;
init_completion(&chan->tx_complete);
@@ -477,7 +477,7 @@ void mbox_free_channel(struct mbox_chan *chan)
/* The queued TX requests are simply aborted, no callbacks are made */
scoped_guard(spinlock_irqsave, &chan->lock) {
chan->cl = NULL;
- chan->active_req = NULL;
+ chan->active_req = MBOX_NO_MSG;
if (chan->txdone_method == TXDONE_BY_ACK)
chan->txdone_method = TXDONE_BY_POLL;
}
@@ -534,6 +534,7 @@ int mbox_controller_register(struct mbox_controller *mbox)
chan->cl = NULL;
chan->mbox = mbox;
+ chan->active_req = MBOX_NO_MSG;
chan->txdone_method = txdone;
spin_lock_init(&chan->lock);
}
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index ff292b9e0be9..7a2baeca2ba4 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -361,7 +361,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
if (pchan->chan.rx_alloc)
handle = write_response(pchan);
- if (chan->active_req) {
+ if (chan->active_req != MBOX_NO_MSG) {
pcc_header = chan->active_req;
if (pcc_header->flags & PCC_CMD_COMPLETION_NOTIFY)
mbox_chan_txdone(chan, 0);
diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index ed9a0bb2bcd8..7991e8dba579 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -497,7 +497,7 @@ static int tegra_hsp_mailbox_flush(struct mbox_chan *chan,
mbox_chan_txdone(chan, 0);
/* Wait until channel is empty */
- if (chan->active_req != NULL)
+ if (chan->active_req != MBOX_NO_MSG)
continue;
return 0;
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index 80a427c7ca29..1db0069c27c5 100644
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -11,6 +11,9 @@
struct mbox_chan;
+/* Sentinel value distinguishing "no active request" from "NULL message data" */
+#define MBOX_NO_MSG ((void *)-1)
+
/**
* struct mbox_chan_ops - methods to control mailbox channels
* @send_data: The API asks the MBOX controller driver, in atomic
--
2.54.0.545.g6539524ca2-goog
next reply other threads:[~2026-05-07 6:21 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-07 6:21 Joonwon Kang [this message]
2026-05-09 2:08 ` [PATCH 6.18.y] mailbox: Fix NULL message support in mbox_send_message() Sasha Levin
2026-05-10 5:50 ` Joonwon Kang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260507062107.2927600-1-joonwonkang@google.com \
--to=joonwonkang@google.com \
--cc=dianders@chromium.org \
--cc=jassisinghbrar@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=sudeep.holla@arm.com \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.