Re: [PATCH v4 28/40] scsi: mvumi: use 16-bits defined sense codes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but failure of `dma_map_sg()` in `mvumi_make_sgl()` is treated as success, leading to submitting malformed commands to the firmware.
--

commit dfc070daf5ae8e3c8f3342502ead3c537cba8d81
Author: Damien Le Moal <dlemoal@kernel.org>

scsi: mvumi: use 16-bits defined sense codes

Refactor the Marvell UMI driver to use scsi_set_sense() and replace all
hard-coded additional sense codes and additional sense code qualifiers
with the enum values defined in include/scsi/scsi_sense.h. This helps with
code clarity as the sense codes being processed are easier to test and
self-documented.

No functional change intended.

> diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
> index e9e2c25cde682..91c2db697c691 100644
> --- a/drivers/scsi/mvumi.c
> +++ b/drivers/scsi/mvumi.c
> @@ -2067,7 +2067,7 @@ static unsigned char mvumi_build_frame(struct mvumi_hba *mhba,
>  	return 0;
>  
>  error:
> -	scsi_build_sense(scmd, 0, ILLEGAL_REQUEST, 0x24, 0);
> +	scsi_set_sense(scmd, 0, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB);

[Severity: High]
This is a pre-existing issue, but does mvumi_make_sgl() treat a
dma_map_sg() failure as a success, leading to submitting malformed
commands to the firmware?

When dma_map_sg() fails (e.g., under memory pressure or IOMMU
exhaustion), it returns 0. The function mvumi_make_sgl() at
drivers/scsi/mvumi.c:198 assigns this to *sg_count and checks if it
is greater than mhba->max_sge:

drivers/scsi/mvumi.c:mvumi_make_sgl() {
    ...
	*sg_count = dma_map_sg(&mhba->pdev->dev, scsi_sglist(scmd), sgnum,
			       scmd->sc_data_direction);
	if (*sg_count > mhba->max_sge) {
    ...
}

Since 0 is not greater than max_sge, the code ignores the error and
returns 0 (success). 

This causes mvumi_build_frame() to submit the command to the firmware
with a non-zero data transfer length but sg_counts == 0.

Could sending these malformed commands to the controller crash the
firmware (DoS), trigger IOMMU faults, or cause data corruption if the
firmware attempts to parse an uninitialized payload as an SG list?

Can this be triggered during heavy I/O loads or when IOMMU mappings are
exhausted locally through the following path?

mvumi_queue_command() -> mvumi_build_frame() -> mvumi_make_sgl()

>  	return -1;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905032224.509860-1-dlemoal@kernel.org?part=28