Re: [PATCH v8 4/7] bus: mhi: Add QDU100 Sahara variant and firmware fallback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mhi: sahara: concurrent workqueues corrupt shared tx[0] DMA buffer
--

commit f8c1fedf3c26d1f3f5b6996e23406ceb42b03572
Author: Kishore Batta <kishore.batta@oss.qualcomm.com>

bus: mhi: Add QDU100 Sahara variant and firmware fallback

This commit extends the Sahara driver to recognize QDU100 devices on the
SAHARA MHI channel and associates it with the QDU100 firmware folder. It also
adds a firmware lookup fallback to allow images to be provisioned by the
platform if they do not have an explicit entry in the device's firmware table.

> diff --git a/drivers/bus/mhi/host/clients/sahara/sahara.c b/drivers/bus/mhi/host/clients/sahara/sahara.c
> index 74bf57f02873e..4baa47d0b94e3 100644
> --- a/drivers/bus/mhi/host/clients/sahara/sahara.c
> +++ b/drivers/bus/mhi/host/clients/sahara/sahara.c

[ ... ]

> @@ -870,8 +891,10 @@ static void sahara_mhi_dl_xfer_cb(struct mhi_device *mhi_dev, struct mhi_result
>  
>  static const struct mhi_device_id sahara_mhi_match_table[] = {
>  	{ .chan = "QAIC_SAHARA", },
> +	{ .chan = "SAHARA", },

[Severity: High]
Does this code have a data race leading to DMA ring corruption?

During a streaming transfer, read_data_work repeatedly uses context->tx[0]
to stream firmware data:

drivers/bus/mhi/host/clients/sahara/sahara.c:read_data_helper() {
    ...
	memcpy(context->tx[buf_index],
	       &context->firmware->data[context->read_data_offset],
	       pkt_data_len);
    ...
}

If the endpoint concurrently sends an asynchronous command like HELLO,
END_OF_IMAGE, or CMD_READY, fw_work is scheduled and will overwrite tx[0]:

drivers/bus/mhi/host/clients/sahara/sahara.c:sahara_hello() {
    ...
	context->tx[0]->cmd = cpu_to_le32(SAHARA_HELLO_RESP_CMD);
	context->tx[0]->length = cpu_to_le32(SAHARA_HELLO_LENGTH);
    ...
}

Since read_data_work and fw_work could modify and queue the same TX buffer
at the same time, is it possible for tx[0] to be overwritten and enqueued
while it is already mapped and active for read_data_work?

An untrusted or misbehaving MHI endpoint could trigger this by sending an
unexpected packet during streaming.

>  	{},
>  };
> +MODULE_DEVICE_TABLE(mhi, sahara_mhi_match_table);
>  
>  static struct mhi_driver sahara_mhi_driver = {
>  	.id_table = sahara_mhi_match_table,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-sahara_protocol_new_v2-v8-0-e12939d229a1@oss.qualcomm.com?part=4