Re: [PATCH v6 26/56] scsi: qla2xxx: Update handling of status entries for 29xx series

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Integer underflow in par_sense_len calculation causes an out-of-bounds read from the DMA response ring buffer.
--

--- Patch [26]: [PATCH v6 26/56] scsi: qla2xxx: Update handling of status entries for 29xx series ---
commit 41d187f5b4a36f6b90c11b4e9bb88831b9152960
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Update handling of status entries for 29xx series

This commit modifies the qla2xxx driver to handle extended status entries 
for the 29xx series. It updates function signatures to accept generic 
pointers and adjusts the parsing logic for FWI2 status IOCBs.

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 0d58dc470e870..5aa45b8f49290 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
[ ... ]
> @@ -3429,20 +3454,20 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
>  	sense_len = par_sense_len = rsp_info_len = resid_len =
>  	    fw_resid_len = 0;
>  	if (IS_FWI2_CAPABLE(ha)) {
> +		qla_sts_fwi2_extract(ha, pkt, &sf);
>  		if (scsi_status & SS_SENSE_LEN_VALID)
> -			sense_len = le32_to_cpu(sts24->sense_len);
> +			sense_len = sf.sense_len;
>  		if (scsi_status & SS_RESPONSE_INFO_LEN_VALID)
> -			rsp_info_len = le32_to_cpu(sts24->rsp_data_len);
> +			rsp_info_len = sf.rsp_data_len;
>  		if (scsi_status & (SS_RESIDUAL_UNDER | SS_RESIDUAL_OVER))
> -			resid_len = le32_to_cpu(sts24->rsp_residual_count);
> +			resid_len = sf.rsp_residual_count;
>  		if (comp_status == CS_DATA_UNDERRUN)
>  			fw_resid_len = le32_to_cpu(sts24->residual_len);
> -		rsp_info = sts24->data;
> -		sense_data = sts24->data;
> -		host_to_fcp_swap(sts24->data, sizeof(sts24->data));
> +		rsp_info = sf.data;
> +		sense_data = sf.data;
> +		par_sense_len = sf.data_sz;

[Severity: High]
This is a pre-existing issue, but can this code cause an integer underflow
when calculating the remaining sense length?

If the HBA firmware returns a status IOCB with SS_RESPONSE_INFO_LEN_VALID set
and provides an rsp_data_len that is larger than the inline data buffer size
in sf.data_sz, the subtraction further down in qla2x00_status_entry() will
wrap around:

	if (scsi_status & SS_RESPONSE_INFO_LEN_VALID) {
		if (IS_FWI2_CAPABLE(ha)) {
			sense_data += rsp_info_len;
			par_sense_len -= rsp_info_len;
		}

Since par_sense_len is an unsigned integer, it becomes a large positive value.
Later, in qla2x00_handle_sense(), this bypasses the bounds check and leads to
an out-of-bounds read from the DMA response ring buffer:

qla2x00_handle_sense() {
    ...
	if (sense_len > par_sense_len)
		sense_len = par_sense_len;

	memcpy(cp->sense_buffer, sense_data, sense_len);
    ...
}

Should rsp_info_len be validated against par_sense_len before subtracting it,
to prevent potential leaks of kernel DMA memory?

> +		sts_qual = sf.sts_qual;
>  		ox_id = le16_to_cpu(sts24->ox_id);
> -		par_sense_len = sizeof(sts24->data);
> -		sts_qual = le16_to_cpu(sts24->status_qualifier);
>  	} else {
>  		if (scsi_status & SS_SENSE_LEN_VALID)
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723050413.3897522-1-njavali@marvell.com?part=26