Re: [PATCH v3 1/2] scsi: libsas: Add PHYE_NOTIFY_ENABLE_SPINUP phy event for ASC/ASCQ=0x04/0x11

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but it is a critical vulnerability. In `sas_ssp_task_response()`, the `min_t` macro incorrectly casts `be32_to_cpu(iu->sense_data_len)` to a signed `int`. If a malicious or malfunctioning SAS device sends a very large `sense_data_len` (e.g., 0xFFFFFFFF), the cast to `int` results in a negative value (e.g., -1). `min_t` evaluates to this negative number, which is stored in `tstat->buf_valid_size`. When passed to `memcpy()`, the negative size is implicitly cast to `size_t`, becoming `SIZE_MAX`. This causes a massive heap buffer overflow when copying from `iu->sense_data`.
--

--- Patch [1]: [PATCH v3 1/2] scsi: libsas: Add PHYE_NOTIFY_ENABLE_SPINUP phy event for ASC/ASCQ=0x04/0x11 ---
commit 04d93ab7400ada2fff2a532b87fdf708e22b793a
Author: Xingui Yang <yangxingui@huawei.com>

scsi: libsas: Add PHYE_NOTIFY_ENABLE_SPINUP phy event for ASC/ASCQ=0x04/0x11

This patch adds a new PHY event to handle ASC/ASCQ 0x04/0x11 for SAS devices
requiring a NOTIFY(ENABLE SPINUP) primitive. It introduces sense detection in
the common task response path and an optional LLDD callback.

> diff --git a/drivers/scsi/libsas/sas_task.c b/drivers/scsi/libsas/sas_task.c
> index e9d291007817b..d1eb6ce90626e 100644
> --- a/drivers/scsi/libsas/sas_task.c
> +++ b/drivers/scsi/libsas/sas_task.c
> @@ -29,6 +29,8 @@ void sas_ssp_task_response(struct device *dev, struct sas_task *task,
>  			      be32_to_cpu(iu->sense_data_len));
>  		memcpy(tstat->buf, iu->sense_data, tstat->buf_valid_size);

[Severity: Critical]
This is a pre-existing issue, but does this min_t() cast to signed int expose
the kernel to a heap buffer overflow?

If a malicious or malfunctioning SAS device returns an SSP response IU with a
very large sense_data_len (e.g., 0xFFFFFFFF), casting it to 'int' results in a
negative value.

This negative value would be stored in tstat->buf_valid_size, and when passed
to memcpy(), it is implicitly cast to size_t, becoming SIZE_MAX.

>  
> +		sas_ssp_task_spinup_notify(task, iu);
> +
>  		if (iu->status != SAM_STAT_CHECK_CONDITION)
>  			dev_warn(dev, "dev %016llx sent sense data, but stat(0x%x) is not CHECK CONDITION\n",
>  				 SAS_ADDR(task->dev->sas_addr), iu->status);

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