From 831c4302961ec42655f802b46002b8ab8ba9732d Mon Sep 17 00:00:00 2001 From: Lina Iyer Date: Fri, 8 Dec 2017 19:36:13 +0000 Subject: [PATCH] drivers: mailbox: fix race resulting in multiple message submission The mailbox sends a request to the controller and the tx_done callback received for that request clears the active_req pointer. The callback sends the next request in the queue, if there is one. When a controller is busy and cannot accept any more requests until the interrupt is cleared, it would return -EAGAIN. The mailbox controller must unlock its spinlock and retry again. An incorrect check for the -EAGAIN allows, the mailbox to send multiple requests without receiving the tx_done callback for the first one sent. As a result, the active_req pointer is overwritten and holds on only to the last request sent. When the second tx_done is received it finds the active_req to be NULL already and bails out before notifying the client. The client may wait forever for this response. Change-Id: Id58c7365be8c6bfc7f90fe9445c88c1246d2d7f8 Signed-off-by: Lina Iyer --- drivers/mailbox/mailbox.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c index a330c6728a56..fb7271b8520c 100644 --- a/drivers/mailbox/mailbox.c +++ b/drivers/mailbox/mailbox.c @@ -53,17 +53,16 @@ static int add_to_rbuf(struct mbox_chan *chan, void *mssg) return idx; } -static void msg_submit(struct mbox_chan *chan) +static int __msg_submit(struct mbox_chan *chan) { unsigned count, idx; unsigned long flags; void *data; int err = -EBUSY; -again: spin_lock_irqsave(&chan->lock, flags); - if (!chan->msg_count || (chan->active_req && err != -EAGAIN)) + if (!chan->msg_count || chan->active_req) goto exit; count = chan->msg_count; @@ -86,15 +85,23 @@ again: exit: spin_unlock_irqrestore(&chan->lock, flags); + return err; +} + +static void msg_submit(struct mbox_chan *chan) +{ + int err = 0; + /* * If the controller returns -EAGAIN, then it means, our spinlock * here is preventing the controller from receiving its interrupt, * that would help clear the controller channels that are currently * blocked waiting on the interrupt response. - * Unlock and retry again. + * Retry again. */ - if (err == -EAGAIN) - goto again; + do { + err = __msg_submit(chan); + } while (err == -EAGAIN); if (!err && (chan->txdone_method & TXDONE_BY_POLL)) /* kick start the timer immediately to avoid delays */