Re: [PATCH v3 1/4] scsi: libsas: Fix out-of-bounds memcpy in sas_ssp_task_response()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: libsas: OOB read in sas_ssp_task_response due to LLDD buffer sizing
- [Medium] scsi: mvsas: info leak in mvs_fill_ssp_resp_iu() due to missing cpu_to_be32
--

commit ff340fa9f4f2ad2be27ec82fd5912bea06443c68
Author: Xingui Yang <yangxingui@huawei.com>

scsi: libsas: Fix out-of-bounds memcpy in sas_ssp_task_response()

This commit changes the min_t clamp of sense_data_len in
sas_ssp_task_response() from int to u32. This prevents negative lengths
from causing out-of-bounds reads and writes during memcpy when the
device provides a value greater than or equal to 0x80000000.

> diff --git a/drivers/scsi/libsas/sas_task.c b/drivers/scsi/libsas/sas_task.c
> index e9d291007817b..2080e5836179b 100644
> --- a/drivers/scsi/libsas/sas_task.c
> +++ b/drivers/scsi/libsas/sas_task.c
> @@ -25,7 +25,7 @@ void sas_ssp_task_response(struct device *dev, struct sas_task *task,
>  	case SAS_DATAPRES_SENSE_DATA:
>  		tstat->stat = SAS_SAM_STAT_CHECK_CONDITION;
>  		tstat->buf_valid_size =
> -			min_t(int, SAS_STATUS_BUF_SIZE,
> +			min_t(u32, SAS_STATUS_BUF_SIZE,
>  			      be32_to_cpu(iu->sense_data_len));

[Severity: Medium]
Does this change inadvertently expose an information leak on little-endian
architectures?

In drivers/scsi/mvsas/mv_sas.c:mvs_fill_ssp_resp_iu(), the driver assigns
the native integer 17 to the __be32 field iu->sense_data_len:

mvs_fill_ssp_resp_iu() {
	...
	iu->sense_data_len = 17;
	...
}

On little-endian architectures, be32_to_cpu() evaluates this as 285212672.
The min_t clamp then evaluates to 96 bytes. Because mvs_set_sense() only
initializes the first 17 bytes, the subsequent memcpy() copies an
additional 79 bytes of uninitialized DMA memory into the task status
buffer.

This uninitialized memory is then copied into the SCSI sense buffer in
drivers/scsi/libsas/sas_scsi_host.c:sas_end_task():

sas_end_task() {
	...
	memcpy(sc->sense_buffer, ts->buf,
	       min(SCSI_SENSE_BUFFERSIZE, ts->buf_valid_size));
	...
}

Could this expose the leaked DMA memory to upper layers and potentially
userspace via the SG_IO ioctl?

>  		memcpy(tstat->buf, iu->sense_data, tstat->buf_valid_size);

[Severity: High]
Can this memcpy read out-of-bounds from the underlying LLDD DMA buffer?

The function clamps the copy length to SAS_STATUS_BUF_SIZE (96 bytes),
but it does not verify if the LLDD actually provided 96 bytes of
physically backed space for iu->sense_data.

For example, in drivers/scsi/pm8001/pm8001_hwi.c:mpi_ssp_completion(),
the ssp_response_iu is embedded in an iomb queue element which is
physically only 64 bytes (or 128 bytes for SPCV). Inside the 64-byte
iomb, the sense_data field starts at offset 44, leaving only 20 bytes
of available space.

If a malicious or malfunctioning SAS target provides a sense_data_len
of 96 or greater, reading 96 bytes here forces an out-of-bounds read
past the end of the iomb queue element.

Could this copy sensitive data from adjacent DMA completion ring
elements (like command tags, statuses, and memory addresses) into the
task status buffer, leading to a kernel memory information leak?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009033238.1076098-1-yangxingui@huawei.com?part=1