Re: [PATCH 2/2] scsi: aacraid: bound the HBA sense response copy to the source buffer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: aacraid: truncation of HBA sense data
--

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

scsi: aacraid: bound the HBA sense response copy to the source buffer

This commit restricts the amount of sense data copied from the native HBA
response into the command's sense buffer. It caps the copy at the size of
the sense_response_buf array to prevent a potential out-of-bounds read.

> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 89d8e33517584..23512648e389e 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
[ ... ]
> @@ -3579,7 +3579,7 @@ static void hba_resp_task_complete(struct aac_dev *dev,
>  		int len;
>  
>  		len = min_t(u8, err->sense_response_data_len,
> -			SCSI_SENSE_BUFFERSIZE);
> +			sizeof(err->sense_response_buf));

[Severity: Medium]
In hba_resp_task_complete(), does this change actually truncate valid
extended sense data returned by the firmware?

Looking at struct aac_native_hba, the err object is backed by a 512-byte
buffer:

union {
	struct aac_hba_resp err;
	u8 resp_bytes[FW_ERROR_BUFFER_SIZE];
} resp;

Because of this union, the original copy of up to SCSI_SENSE_BUFFERSIZE (96
bytes) appears to have been memory-safe. Capping the length to the size of
sense_response_buf limits the copy to 32 bytes. This would discard diagnostic
information whenever the HBA firmware returns a CHECK CONDITION status with
more than 32 bytes of sense data.

>  		if (len)
>  			memcpy(scsicmd->sense_buffer,
>  				err->sense_response_buf, len);

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