Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Missing cancellation of `read_data_work` during device removal leads to a Use-After-Free. - [High] Lack of synchronization between concurrent device command processing and data streaming leads to Use-After-Free and TX buffer corruption. -- commit 812426ed0db5b6e824926f5d8eecb24c749a578e Author: Kishore Batta <[email protected]> bus: mhi: Centralize Sahara firmware image table selection at probe time This commit refactors the Sahara driver to select firmware image tables using a controller provided mapping table at probe time. It centralizes device specific conditionals that were previously scattered across the driver. This prepares the driver for extending support to new devices. [ ... ] > diff --git a/drivers/bus/mhi/host/clients/sahara/sahara.c > b/drivers/bus/mhi/host/clients/sahara/sahara.c > index 858dc5bc39c1a..e339c67e236af 100644 > --- a/drivers/bus/mhi/host/clients/sahara/sahara.c > +++ b/drivers/bus/mhi/host/clients/sahara/sahara.c > @@ -179,48 +179,7 @@ struct sahara_context { > u32 read_data_length; > bool is_mem_dump_mode; > bool non_streaming; [ ... ] > + const char *fw_folder; > }; [Severity: High] This is a pre-existing issue, but can the device removal path cause a use-after-free here? In drivers/bus/mhi/host/clients/sahara/sahara.c:sahara_mhi_remove(), the teardown cancels other work items: cancel_work_sync(&context->fw_work); cancel_work_sync(&context->dump_work); cancel_work_sync(&context->cmd_work); However, it seems cancel_work_sync(&context->read_data_work) is omitted. If a device in streaming mode is actively transferring firmware data via read_data_work, and the driver is unbound, devres will free the device-managed context structure. Will the pending or sleeping sahara_read_data_processing() work item then continue to run and access the freed context and its firmware buffer? [Severity: High] This is a pre-existing issue, but is there a lack of synchronization between concurrent device command processing and data streaming that could corrupt the TX buffer? If read_data_work is running to stream a firmware image, and a faulty or compromised device sends an out-of-band message (like END_OF_IMAGE_CMD) on the downlink channel, fw_work will run concurrently with read_data_work. fw_work calls sahara_end_of_image(), which frees context->firmware and writes a response to context->tx[0]: sahara_release_image(context); ... context->tx[0]->cmd = cpu_to_le32(SAHARA_DONE_CMD); Concurrently, read_data_helper() executed from read_data_work accesses the freed firmware and writes to the same TX buffer: memcpy(context->tx[buf_index], &context->firmware->data[context->read_data_offset], pkt_data_len); Does this sequence result in a use-after-free and packet corruption before queuing it to MHI? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260701-sahara_protocol_new_v2-v6-0-3a78362c4...@oss.qualcomm.com?part=3
