Re: [PATCH v6 49/56] scsi: qla2xxx: Check entry_status in qla24xx_modify_vp_config()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing IOCB `entry_status` checks cause firmware rejections to be incorrectly treated as success.
--

--- Patch [49]: [PATCH v6 49/56] scsi: qla2xxx: Check entry_status in qla24xx_modify_vp_config() ---
commit 2e991988fb5984a6e00b391f35371b9211fc9e9a
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Check entry_status in qla24xx_modify_vp_config()

This commit fixes a bug in qla24xx_modify_vp_config() where the IOCB
completion handler checked comp_status instead of entry_status. This
prevented early firmware rejections from being properly detected.

> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -4401,10 +4401,10 @@ qla24xx_modify_vp_config(scsi_qla_host_t *vha)
>  	if (rval != QLA_SUCCESS) {
>  		ql_dbg(ql_dbg_mbx, vha, 0x10bd,
>  		    "Failed to issue VP config IOCB (%x).\n", rval);
> -	} else if (vpmod->comp_status != 0) {
> +	} else if (vpmod->entry_status != 0) {
>  		ql_dbg(ql_dbg_mbx, vha, 0x10be,
>  		    "Failed to complete IOCB -- error status (%x).\n",
> -		    vpmod->comp_status);
> +		    vpmod->entry_status);
>  		rval = QLA_FUNCTION_FAILED;
>  	} else if (vpmod->comp_status != cpu_to_le16(CS_COMPLETE)) {

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing the
missing entry_status check here, do we have similar vulnerabilities in other
IOCB handlers across the driver?

Looking at other parts of the driver, several functions appear to make the
same mistake of ignoring entry_status when an IOCB is rejected early.

For example, in qla84xx_verify_chip(), it only checks comp_status:

drivers/scsi/qla2xxx/qla_mbx.c:qla84xx_verify_chip() {
    ...
    status[0] = le16_to_cpu(mn->p.rsp.comp_status);
    status[1] = status[0] == CS_VCS_CHIP_FAILURE ?
        le16_to_cpu(mn->p.rsp.failure_code) : 0;
    ...
    if (status[0] != CS_COMPLETE) {
    ...
}

Could this treat a rejected verify chip IOCB as a success if entry_status
is set but comp_status remains zero?

Similarly, the BSG handlers qla84xx_updatefw() and qla84xx_mgmt_cmd() in
drivers/scsi/qla2xxx/qla_bsg.c only check the mailbox command return
status (rval) but ignore both entry_status and comp_status entirely.

Do these need to be updated to prevent returning DID_OK to userspace if
the firmware update fails, or copying zeroed memory into the user's BSG
reply payload?

Finally, in the ELS/ABTS paths, qla24xx_process_abts() in qla_isr.c and
qla24xx_process_purex_rdp() in qla_os.c both check rsp_els->comp_status
but completely ignore entry_status.

Could a rejected exchange termination be silently treated as successful
here, leaving active exchanges in the firmware and potentially causing
resource exhaustion?

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