Re: [PATCH v3 39/88] scsi: qla2xxx: Adjust feature gating in BSG paths for 29xx support

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Incomplete Feature Enablement: D-port diagnostics v2 API is not enabled for QLA29xx adapters because the IS_DPORT_CAPABLE macro was not updated, causing the command to unconditionally reject QLA29xx.

Pre-existing issues:
- [Critical] Information Leak: The function qla2x00_do_dport_diagnostics() leaks up to 62 bytes of uninitialized kernel heap memory to user space.
- [High] Resource Leak / Hanging Process: BSG job hangs indefinitely on validation error because `bsg_job_done()` is skipped but the function returns `QLA_SUCCESS`.
--

--- Patch [39]: [PATCH v3 39/88] scsi: qla2xxx: Adjust feature gating in BSG paths for 29xx support ---
commit 9bba08258f3cd9b122f24b8370fef88669196b77
Author: Manish Rangankar <mrangankar@marvell.com>

scsi: qla2xxx: Adjust feature gating in BSG paths for 29xx support

Extend qla2xxx BSG command handling to recognize QLA29xx adapters and
align feature availability with hardware capabilities.

> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index 8a969174a261e..fade3638d31cb 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c

[ ... ]

> @@ -2789,7 +2789,7 @@ qla2x00_do_dport_diagnostics(struct bsg_job *bsg_job)
>  	struct qla_dport_diag *dd;
>  
>  	if (!IS_QLA83XX(vha->hw) && !IS_QLA27XX(vha->hw) &&
> -	    !IS_QLA28XX(vha->hw))
> +	    !IS_QLA28XX(vha->hw) && !IS_QLA29XX(vha->hw))
>  		return -EPERM;

[Severity: Medium]
Does enabling QLA29xx here leave out the v2 diagnostics interface?

Looking at qla2x00_do_dport_diagnostics_v2() in the same file, it relies on
the IS_DPORT_CAPABLE macro:

    if (!IS_DPORT_CAPABLE(vha->hw))
            return -EPERM;

Since IS_DPORT_CAPABLE in qla_def.h only checks for 83XX, 27XX, and 28XX,
won't qla2x00_do_dport_diagnostics_v2() incorrectly reject QLA29xx adapters
and fail with -EPERM?

[Severity: Critical]
This is a pre-existing issue, but does enabling this function for QLA29xx
expose an information leak?

In qla2x00_do_dport_diagnostics(), the dd structure is allocated without
zeroing the memory:

    dd = kmalloc_obj(*dd);

Later, sg_copy_to_buffer() only overwrites the portion of dd provided by
the user's request payload, which can leave bytes uninitialized (such as
the unused fields). Finally, sg_copy_from_buffer() copies the entire
sizeof(*dd) back to the reply payload:

    sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
        bsg_job->reply_payload.sg_cnt, dd, sizeof(*dd));

Could this leak uninitialized kernel heap memory back to user space?

[ ... ]

> @@ -2915,8 +2915,13 @@ qla2x00_get_flash_image_status(struct bsg_job *bsg_job)
>  	struct qla_active_regions regions = { };
>  	struct active_regions active_regions = { };
>  
> -	qla27xx_get_active_image(vha, &active_regions);
> -	regions.global_image = active_regions.global;
> +	if (IS_QLA29XX(ha))
> +		return -EPERM;
> +
> +	if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
> +		qla27xx_get_active_image(vha, &active_regions);
> +		regions.global_image = active_regions.global;
> +	}

[Severity: High]
This is a pre-existing issue, but while reviewing the flash image handling
in this file, I noticed a potential hang in qla28xx_validate_flash_image().

When validation fails and rval is non-zero, the error path explicitly skips
calling bsg_job_done() but still returns QLA_SUCCESS:

    if (!rval)
            bsg_job_done(bsg_job, bsg_reply->result,
                         bsg_reply->reply_payload_rcv_len);

    return QLA_SUCCESS;

Since the BSG core expects bsg_job_done() to be called if the driver
returns a success code, won't this cause the user space process to hang
indefinitely waiting for the job to complete?

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