Re: [PATCH v5 54/56] scsi: qla2xxx: Fix BSG job leak on validate flash image error path

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pervasive slab out-of-bounds reads when accessing BSG vendor_cmd payload due to missing request_len checks.
--

--- Patch [54]: [PATCH v5 54/56] scsi: qla2xxx: Fix BSG job leak on validate flash image error path ---
commit f84362ea09312f70b70c60c0ecfaeb501ca572d1
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Fix BSG job leak on validate flash image error path

qla28xx_validate_flash_image() returns QLA_SUCCESS unconditionally, telling
the FC BSG transport that the driver owns and will complete the request.
However, bsg_job_done() was guarded by an error check, causing requests to
dangle on error paths until timeout. This commit removes the error check
to ensure the job is always completed.

> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index 3bf3a7b96eb50..2f73c9af418d7 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -3848,9 +3848,8 @@ static int qla28xx_validate_flash_image(struct bsg_job *bsg_job)
>  	bsg_reply->result = DID_OK << 16;
>  	bsg_reply->reply_payload_rcv_len = 0;
>  	bsg_job->reply_len = sizeof(struct fc_bsg_reply);
> -	if (!rval)
> -		bsg_job_done(bsg_job, bsg_reply->result,
> -			     bsg_reply->reply_payload_rcv_len);
> +	bsg_job_done(bsg_job, bsg_reply->result,
> +		     bsg_reply->reply_payload_rcv_len);

[Severity: High]
This isn't a bug introduced by this patch, but there is a pervasive slab
out-of-bounds read issue when accessing the BSG vendor_cmd payload due to
missing request_len checks.

Is it possible for a privileged user (with CAP_SYS_RAWIO) to issue a short
BSG command with a small request_len (e.g. 4 or 8 bytes), causing the driver
to read out-of-bounds slab memory?

For example, when this function calls qla28xx_do_validate_flash_image(),
the command reads vendor_cmd[1] unconditionally:

qla28xx_do_validate_flash_image() {
    ...
    img_idx = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
    ...
}

Similarly, earlier in the BSG path, qla24xx_bsg_request() unconditionally
reads vendor_cmd[0] before checking if the request length covers it:

qla24xx_bsg_request() {
    ...
    if (bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_MANAGE_HOST_PORT ||
        bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_GET_HOST_STATS)
    ...
}

Should the transport layer or the driver validate that the user's length is
large enough before accessing these vendor command array elements to avoid
a KASAN panic or executing arbitrary driver logic with uninitialized memory?

>  
>  	return QLA_SUCCESS;
>  }

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