On Tue, Aug 17, 2021 at 3:01 AM Xianting TIan
<xianting.tian@xxxxxxxxxxxxxxxxx> wrote:
As I said, send_data() is basically "accepted for transfer", and not
在 2021/8/17 下午1:58, Xianting TIan 写道:
在 2021/8/17 下午12:29, Jassi Brar 写道:
On Fri, Aug 6, 2021 at 7:15 AM Xianting TianThanks for the comments,
<xianting.tian@xxxxxxxxxxxxxxxxx> wrote:
We met a UAF issue during our mailbox testing.Seems like your controller's .send_data() returns error. Can you
In synchronous mailbox, we use mbox_send_message() to send a message
and wait for completion. mbox_send_message() calls msg_submit() to
send the message for the first time, if timeout, it will send the
message in tx_tick() for the second time.
please explain why it does so? Because
send_data() only _accepts_ data for further transmission... which
should seldom be a problem.
We developed virtio-mailbox for heterogeneous virtualization system.
virtio-mailbox is based on the mialbox framework.
In virtio framework, its send func 'virtqueue_add_outbuf()' may return
error, which caused .send_data() return error. And also contains
other csenarios.
But I think mailbox framework shouldn't depend on .send_data() always
return OK, as .send_data() is implemented by mailbox hardware
manufacturer, which is not controlled by mailbox framework itself.
"transferred fine".
The api is used not just in synchronous mode.You said 'seldom', but it still possible we can meet such issue. sucn
as flexrm_send_data() of drivers/mailbox/bcm-flexrm-mailbox.c.
And the flexrm driver relies on ACK method, not the synchronous one.
Normal is your controller driver should be ready after .startup() andI think mailbox framework should be work normaly no matter
.send_data() returns ok or not ok. Do you think so? thanks
accepts the first message submitted to it.
If it does that, everything would work.
Another solution is to ignore the return value of .send_data() inYes, I also have something like this in mind. Definitely not the hack
msg_submit(),
change
err = chan->mbox->ops->send_data(chan, data);
if (!err) {
chan->active_req = data;
chan->msg_count--;
}
to
chan->mbox->ops->send_data(chan, data);
chan->active_req = data;
chan->msg_count--;
in your original post.
But the side effect of the solution is obvious, as if send failed in theThe api doesn't retry. The .last_tx_done() is supposed to tell the
first time, it will have no chance to sent it in tx_tick() for the
second time. That is to say, no retry.
api when is it ok to send a message.
The following should work for you (though I believe your code needs
fixing), which anyway, should have been there.
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 3e7d4b20ab34..9824a51b82fa 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -75,10 +75,12 @@ static void msg_submit(struct mbox_chan *chan)
chan->cl->tx_prepare(chan->cl, data);
/* Try to submit a message to the MBOX controller */
err = chan->mbox->ops->send_data(chan, data);
- if (!err) {
+ if (!err)
chan->active_req = data;
- chan->msg_count--;
- }
+
+ /* done with another message */
+ chan->msg_count--;
+
exit:
spin_unlock_irqrestore(&chan->lock, flags);