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 <ilina@codeaurora.org>
This commit is contained in:
Lina Iyer 2017-12-08 19:36:13 +00:00 committed by Gerrit - the friendly Code Review server
parent 3008f90010
commit 831c430296

View File

@ -53,17 +53,16 @@ static int add_to_rbuf(struct mbox_chan *chan, void *mssg)
return idx; return idx;
} }
static void msg_submit(struct mbox_chan *chan) static int __msg_submit(struct mbox_chan *chan)
{ {
unsigned count, idx; unsigned count, idx;
unsigned long flags; unsigned long flags;
void *data; void *data;
int err = -EBUSY; int err = -EBUSY;
again:
spin_lock_irqsave(&chan->lock, flags); spin_lock_irqsave(&chan->lock, flags);
if (!chan->msg_count || (chan->active_req && err != -EAGAIN)) if (!chan->msg_count || chan->active_req)
goto exit; goto exit;
count = chan->msg_count; count = chan->msg_count;
@ -86,15 +85,23 @@ again:
exit: exit:
spin_unlock_irqrestore(&chan->lock, flags); 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 * If the controller returns -EAGAIN, then it means, our spinlock
* here is preventing the controller from receiving its interrupt, * here is preventing the controller from receiving its interrupt,
* that would help clear the controller channels that are currently * that would help clear the controller channels that are currently
* blocked waiting on the interrupt response. * blocked waiting on the interrupt response.
* Unlock and retry again. * Retry again.
*/ */
if (err == -EAGAIN) do {
goto again; err = __msg_submit(chan);
} while (err == -EAGAIN);
if (!err && (chan->txdone_method & TXDONE_BY_POLL)) if (!err && (chan->txdone_method & TXDONE_BY_POLL))
/* kick start the timer immediately to avoid delays */ /* kick start the timer immediately to avoid delays */