Re: [PATCH v9 7/9] scsi: mvumi: free the internal command buffer by what was allocated

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: mvumi: DMA Use-After-Free on internal command timeout
--

commit a045f4b7ba7499dba25350d8e9d333f7c9956722
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

scsi: mvumi: free the internal command buffer by what was allocated
    
This patch stores the DMA buffer address and length directly in the
mvumi_cmd struct instead of reconstructing it from the command frame.
This avoids an issue where a 32-bit shift in mvumi_delete_internal_cmd()
caused the upper 32 bits of the DMA address to be truncated.

> diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
> index 2410d2aa147d9..133e53bc2cb37 100644
> --- a/drivers/scsi/mvumi.c
> +++ b/drivers/scsi/mvumi.c
[ ... ]
> @@ -286,21 +288,10 @@ static struct mvumi_cmd *mvumi_create_internal_cmd(struct mvumi_hba *mhba,
>  static void mvumi_delete_internal_cmd(struct mvumi_hba *mhba,
>  						struct mvumi_cmd *cmd)
>  {
> -	struct mvumi_sgl *m_sg;
> -	unsigned int size;
> -	dma_addr_t phy_addr;
> -
>  	if (cmd && cmd->frame) {
> -		if (cmd->frame->sg_counts) {
> -			m_sg = (struct mvumi_sgl *) &cmd->frame->payload[0];
> -			sgd_getsz(mhba, m_sg, size);
> -
> -			phy_addr = (dma_addr_t) m_sg->baseaddr_l |
> -				(dma_addr_t) ((m_sg->baseaddr_h << 16) << 16);
> -
> -			dma_free_coherent(&mhba->pdev->dev, size, cmd->data_buf,
> -								phy_addr);
> -		}
> +		if (cmd->data_buf)
> +			dma_free_coherent(&mhba->pdev->dev, cmd->data_buf_size,
> +					  cmd->data_buf, cmd->data_buf_phys);

[Severity: High]
If an internal command times out in mvumi_issue_blocked_cmd(), the function
returns without explicitly aborting the hardware operation or resetting the
controller.

The callers of mvumi_issue_blocked_cmd() (such as mvumi_inquiry(),
mvumi_flush_cache(), or mvumi_scan_events()) then immediately invoke
mvumi_delete_internal_cmd(), which unconditionally frees the DMA-mapped
command frame and data buffer back to the system.

Since the hardware is unaware that the software has abandoned the command,
can a delayed completion cause the hardware to write to the freed physical
memory, resulting in a DMA use-after-free and memory corruption?

>  		dma_free_coherent(&mhba->pdev->dev, mhba->ib_max_size,
>  				cmd->frame, cmd->frame_phys);
>  		kfree(cmd);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015045213.3975845-1-yhlee@isslab.korea.ac.kr?part=7