Re: [PATCH v2 29/60] 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] An attacker-controlled target can cause an integer underflow in `par_sense_len` resulting in an out-of-bounds read of adjacent kernel heap/DMA ring memory, leaking it to user-space.
--

commit f656a1805f0e016da73a51f8736cdf5930510fd1
Author: Nilesh Javali <njavali@marvell.com>

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

This commit modifies the handling of status entries in the qla2xxx driver to
accommodate the extended structure for the 29xx series. It updates function
signatures to accept generic pointers and adjusts logic to differentiate
between standard and extended status entries.

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index c18ee2459f5bd..f81cf70a0542c 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c

[ ... ]

> @@ -3480,20 +3505,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;

[Severity: High]
This is a pre-existing issue, but can an attacker-controlled target cause an
integer underflow here?

If a compromised target provides an rsp_info_len (via sf.rsp_data_len)
larger than the actual data buffer size (par_sense_len, initialized to 28
or 60), the subtraction later in qla2x00_status_entry():

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

will underflow par_sense_len to a massive positive value.

Subsequently, when qla2x00_handle_sense() is called, the length check:

    if (sense_len > par_sense_len)
        sense_len = par_sense_len;

fails to cap sense_len, allowing the following memcpy:

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

to read out-of-bounds adjacent kernel heap or DMA ring memory, leaking it
into the SCSI command's sense buffer.

>  		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;

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