Re: [PATCH RFC 01/10] mailbox: add direct synchronous send support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mailbox: missing validation of send_data allows NULL dereference
--

commit 4e6ec78e9c624fa338c55d686383cecee65b8cdb
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

mailbox: add direct synchronous send support

Some mailbox controllers can complete a transaction entirely within
the calling context, without going through the queued TX state
machine or a TX-done interrupt. Add a synchronous send path so their
clients can request one and wait for the result directly.

> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index efacd24a085d1..c640b19de6080 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c

[ ... ]

> @@ -355,6 +412,14 @@ static int __mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl)
>  	struct device *dev = cl->dev;
>  	int ret;
>  
> +	if (cl->tx_sync) {
> +		if (!chan->mbox->ops->send_data_sync)
> +			return -EOPNOTSUPP;
> +
> +		if (cl->tx_block || cl->tx_tout || cl->knows_txdone)
> +			return -EINVAL;
> +	}
> +

[Severity: Medium]
If a controller only provides send_data_sync (a synchronous-only controller),
and an asynchronous client without cl->tx_sync set binds to it, shouldn't we
also validate the presence of the legacy send_data op?

If an async client binds to a synchronous-only controller, it appears it
could later result in a NULL pointer dereference when sending a message:

drivers/mailbox/mailbox.c:msg_submit() {
    ...
    err = chan->mbox->ops->send_data(chan, data);
    ...
}

>  	if (chan->cl || !try_module_get(chan->mbox->dev->driver->owner)) {
>  		dev_err(dev, "%s: mailbox not free\n", __func__);
>  		return -EBUSY;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-riscv-rpmi-tee-abi-v1-0-04908b81d885@oss.qualcomm.com?part=1