mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-05-16 18:22:00 -04:00
mailbox: Fix NULL message support in mbox_send_message()
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>
This commit is contained in:
@@ -50,7 +50,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;
|
||||
@@ -85,13 +85,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 */
|
||||
@@ -112,7 +112,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);
|
||||
@@ -267,7 +267,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);
|
||||
@@ -340,7 +340,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);
|
||||
|
||||
@@ -498,7 +498,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;
|
||||
}
|
||||
@@ -553,6 +553,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);
|
||||
}
|
||||
|
||||
@@ -495,7 +495,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;
|
||||
|
||||
@@ -12,6 +12,9 @@
|
||||
|
||||
struct mbox_chan;
|
||||
|
||||
/* Sentinel value distinguishing "no active request" from "NULL message data" */
|
||||
#define MBOX_NO_MSG ((void *)-1)
|
||||
|
||||
#define TXDONE_BY_IRQ BIT(0) /* controller has remote RTR irq */
|
||||
#define TXDONE_BY_POLL BIT(1) /* controller can read status of last TX */
|
||||
#define TXDONE_BY_ACK BIT(2) /* S/W ACK received by Client ticks the TX */
|
||||
|
||||
Reference in New Issue
Block a user