Re: [PATCH 07/37] scsi: mpt3sas: Validate SMP request/response length in bsg handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: mpt3sas: zero-length SGE in _transport_smp_handler() for 4-byte buffers
--

commit 64327b5ec9fbcb353b873461686c34d1c93bb8da
Author: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>

scsi: mpt3sas: Validate SMP request/response length in bsg handler

This commit adds a length check in _transport_smp_handler() to reject SMP
requests whose mapped request or response buffer is smaller than 4 bytes,
preventing a length underflow when subtracting the 4-byte CRC.

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c b/drivers/scsi/mpt3sas/mpt3sas_transport.c
> index e74a526efa8db..abb46a5e1c1af 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c
> @@ -2095,6 +2095,13 @@ _transport_smp_handler(struct bsg_job *job, struct Scsi_Host *shost,
>  	if (rc)
>  		goto unmap_out;
>  
> +	if (dma_len_out < 4 || dma_len_in < 4) {
> +		ioc_err(ioc, "%s: SMP request or response too small\n",
> +			__func__);
> +		rc = -EINVAL;
> +		goto unmap_in;
> +	}

[Severity: Medium]
What happens if dma_len_out == 4 or dma_len_in == 4?

Later in _transport_smp_handler(), 4 bytes are subtracted for the trailing
CRC when setting RequestDataLength and calling ioc->build_sg():

drivers/scsi/mpt3sas/mpt3sas_transport.c:_transport_smp_handler() {
	...
	mpi_request->RequestDataLength = cpu_to_le16(dma_len_out - 4);
	psge = &mpi_request->SGL;

	ioc->build_sg(ioc, psge, dma_addr_out, dma_len_out - 4, dma_addr_in,
			dma_len_in - 4);
	...
}

When either buffer is exactly 4 bytes, data_out_sz or data_in_sz is passed as
0 to _base_build_sg() or _base_build_sg_ieee() in
drivers/scsi/mpt3sas/mpt3sas_base.c:

	if (!data_out_sz && !data_in_sz) {
		_base_build_zero_len_sge_ieee(ioc, psge);
		return;
	}

	if (data_out_sz && data_in_sz) {
		...
	}

Because both _base_build_sg() and _base_build_sg_ieee() only construct the
two-element (outbound request + inbound response) SGL required by
MPI2_FUNCTION_SMP_PASSTHROUGH when both sizes are non-zero, a 4-byte buffer
skips the two-SGE branch and submits a single SGE to the firmware (for
example, placing the inbound response SGE into the first outbound request SGE
slot when dma_len_out == 4 and dma_len_in > 4).

Should this check reject buffers with <= 4 bytes (or < 8 bytes to account for
the 4-byte SMP header and 4-byte CRC) so that a zero-length SGE is never
passed to ioc->build_sg()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/1793925772-1303-1-git-send-email-shivasharan.srikanteshwara@broadcom.com?part=7