Re: [PATCH v5 39/56] 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:

Pre-existing issues:
- [High] The `qla2x00_do_dport_diagnostics` BSG handler leaks up to 128 bytes of uninitialized kernel heap memory to userspace.
- [High] The D-port diagnostics function passes an unaligned inner struct field to `dma_map_single()` and subsequently modifies it with `memset()` after it has been mapped for `DMA_FROM_DEVICE`, violating DMA API rules and risking memory corruption.
- [High] The `qla2x00_update_fru_versions` BSG handler writes uninitialized kernel stack memory to the adapter's persistent storage (NVRAM/flash) and can cause a kernel panic when provided a malformed request.
--

--- Patch [39]: [PATCH v5 39/56] scsi: qla2xxx: Adjust feature gating in BSG paths for 29xx support ---
commit e379ff9d45844a3d1de7b4db9393d48209a2ed9c
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 46eed5df7eefb..9ee56ccd52b01 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -2014,7 +2014,7 @@ static int qla2900_bsg_load_mpi(struct bsg_job *bsg_job)
>  	bsg_job_done(bsg_job, bsg_reply->result,
>  		     bsg_reply->reply_payload_rcv_len);
>  
> -	return rval;
> +	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does qla2x00_update_fru_versions()
in drivers/scsi/qla2xxx/qla_bsg.c have an out-of-bounds read?

If a user provides a small payload but specifies a large count, the loop:

while (count--) {
    memcpy(sfp, &image->field_info, sizeof(image->field_info));

will iterate over uninitialized trailing stack memory in the bsg array
since there is no bounds check on count against the payload size.
Could this leak kernel stack memory to hardware storage or cause a panic?

[ ... ]

> @@ -2855,7 +2855,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;
>  
>  	dd = kmalloc_obj(*dd);

[Severity: High]
This isn't a bug introduced by this patch, but by allowing QLA29xx adapters
here, does qla2x00_do_dport_diagnostics() leak kernel heap memory?

The dd buffer is allocated without zero-initialization. If a user provides
a short request payload (e.g., 0 bytes), trailing fields like dd->options
and the 62-byte unused array remain uninitialized.

When sg_copy_from_buffer() copies the full 128 bytes back to the user's
reply payload, could this disclose uninitialized kernel heap memory?

[ ... ]

> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index cec308811d9eb..ce845afe3e16a 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -6566,7 +6566,7 @@ qla26xx_dport_diagnostics(scsi_qla_host_t *vha,
>  	dma_addr_t dd_dma;
>  
>  	if (!IS_QLA83XX(vha->hw) && !IS_QLA27XX(vha->hw) &&
> -	    !IS_QLA28XX(vha->hw))
> +	    !IS_QLA28XX(vha->hw) && !IS_QLA29XX(vha->hw))
>  		return QLA_FUNCTION_FAILED;
>  
>  	ql_dbg(ql_dbg_mbx + ql_dbg_verbose, vha, 0x119f,

[Severity: High]
This isn't a bug introduced by this patch, but does exposing this to QLA29xx
adapters risk memory corruption via DMA API violations in
qla26xx_dport_diagnostics() in drivers/scsi/qla2xxx/qla_mbx.c?

The struct qla_dport_diag is packed, placing its buf array at an unaligned
offset. When this is mapped:

dd_dma = dma_map_single(&vha->hw->pdev->dev, dd_buf, size, DMA_FROM_DEVICE);

Mapping an unaligned address violates DMA rules. Furthermore, immediately
after mapping, the memory is modified:

memset(dd_buf, 0, size);

Does writing to memory while mapped for DMA_FROM_DEVICE dirty the CPU cache,
which could overwrite the hardware DMA results upon eviction?

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