Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b7c3ced1 authored by Lina Iyer's avatar Lina Iyer
Browse files

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: default avatarLina Iyer <ilina@codeaurora.org>
parent 2fed71ce

drivers/mailbox/mailbox.c

+13 −6
Original line number Diff line number Diff line
@@ -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 @@ static void msg_submit(struct mbox_chan *chan) 
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 */