Re: [PATCH v3 15/88] scsi: qla2xxx: Extend execute_fw mailbox to include 29xx

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Structure mismatch when preparing CT/FDMI MS IOCBs for QLA29XX adapters results in firmware executing wild DMAs using uninitialized garbage addresses.
--

--- Patch [15]: [PATCH v3 15/88] scsi: qla2xxx: Extend execute_fw mailbox to include 29xx ---
commit 29bbcde0cc31e2b4641ba7302984a3c7980dc0ef
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Extend execute_fw mailbox to include 29xx

Add IS_QLA29XX() to the BPM capability macros and to the
execute-firmware mailbox command so that NVMe enable, minimum
speed negotiation, 128 Gbps speed reporting, EDIF hardware
detection, and FW-semaphore retry logic all apply to 29xx
adapters.

> 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

[ ... ]

> @@ -727,14 +728,13 @@ qla2x00_execute_fw(scsi_qla_host_t *vha, uint32_t risc_addr)
>  				    ha->lr_distance << LR_DIST_FW_POS;
>  		}
>  
> -		if (ql2xnvmeenable && (IS_QLA27XX(ha) || IS_QLA28XX(ha)))
> +		if (ql2xnvmeenable && (IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)))
>  			mcp->mb[4] |= NVME_ENABLE_FLAG;
>  
> -		if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
> -			struct nvram_81xx *nv = ha->nvram;
> +		if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) {

[Severity: Critical]
Since this patch enables initialization and capabilities for 29xx adapters,
does it also need to update CT/FDMI MS IOCB preparation to use the 128-byte
extended format required by QLA29XX hardware?

When the driver sends FDMI registrations or CT pass-through commands on
QLA29xx hardware, it unconditionally prepares the buffer using the legacy
64-byte struct ct_entry_24xx format. For example, in
qla24xx_prep_ms_fdmi_iocb():

drivers/scsi/qla2xxx/qla_gs.c:qla24xx_prep_ms_fdmi_iocb() {
    ...
    ct_pkt = (struct ct_entry_24xx *)ha->ms_iocb;
    memset(ct_pkt, 0, sizeof(struct ct_entry_24xx));
    ...
}

And similarly in qla24xx_prep_ms_iocb():

drivers/scsi/qla2xxx/qla_gs.c:qla24xx_prep_ms_iocb() {
    ...
    ct_pkt = (struct ct_entry_24xx *)arg->iocb;
    memset(ct_pkt, 0, sizeof(struct ct_entry_24xx));
    ...
}

QLA29XX firmware expects the 128-byte struct ct_entry_24xx_ext format,
where lengths are at offsets 64/68 and DSDs begin at offset 72. Because
ha->ms_iocb is allocated via dma_pool_alloc without __GFP_ZERO, offsets
beyond 64 contain uninitialized memory. 

Can this cause the 29xx firmware to read garbage lengths and DSD DMA addresses
and perform wild DMAs, leading to arbitrary memory corruption or HBA firmware
panics during adapter initialization?

>  			/* set minimum speed if specified in nvram */
>  			if (nv->min_supported_speed >= 2 &&
> -			    nv->min_supported_speed <= 5) {
> +			    nv->min_supported_speed <= 7) {

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