Re: [PATCH v2 37/60] scsi: qla2xxx: Enhance ABTS processing for 29xx series

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Releasing and reusing the `rsp_pkt` DMA buffer immediately after a hardware timeout can lead to a use-after-free and memory corruption by the device.
--

commit 95311a058967d0d565bd44ef588d75babbb03f6f
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Enhance ABTS processing for 29xx series

This commit introduces extended ABTS entry structures to properly
handle the 128-byte IOCB format for the 29xx series adapters. It also
adds type-generic macros to reduce code duplication in the ABTS
processing path.

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 5b5ccf2075353..284d874392fd5 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
[ ... ]
> @@ -140,60 +212,27 @@ qla24xx_process_abts(struct scsi_qla_host *vha, struct purex_item *pkt)
>  	}
>  
>  	/* send ABTS response */
> -	abts_rsp = rsp_pkt;
> -	memset(abts_rsp, 0, sizeof(*abts_rsp));
[ ... ]
> -	rval = qla2x00_issue_iocb(vha, abts_rsp, dma, 0);
> -	if (rval) {
[ ... ]
> +	if (IS_QLA29XX(ha)) {
> +		struct abts_entry_24xx_ext *rsp_ext = rsp_pkt;
> +
> +		QLA_BUILD_ABTS_BA_ACC(rsp_ext, abts_ext,
> +		    abts_ext->sof_type, fctl);
> +		QLA_LOG_ISSUE_ABTS_RSP(vha, rsp_ext, dma, rval);
>  	} else {
> -		ql_dbg(ql_dbg_init, vha, 0x028ea,
> -		    "%s: done.\n", __func__);
> +		struct abts_entry_24xx *abts_rsp = rsp_pkt;
> +
> +		QLA_BUILD_ABTS_BA_ACC(abts_rsp, abts,
> +		    abts->sof_type & 0xf0, fctl);
> +		QLA_LOG_ISSUE_ABTS_RSP(vha, abts_rsp, dma, rval);
>  	}
>  
>  	dma_free_coherent(&ha->pdev->dev, rsp_sz, rsp_pkt, dma);
>  }

[Severity: High]
This is a pre-existing issue, but could this code lead to a use-after-free
and memory corruption?

Earlier in qla24xx_process_abts(), qla2x00_issue_iocb() is called to send an
ELS response using the same rsp_pkt buffer:

    rval = qla2x00_issue_iocb(vha, rsp_pkt, dma, 0);
    if (rval) {
        ql_log(ql_log_warn, vha, 0x0288,
            "%s: iocb failed to execute -> %x\n", __func__, rval);
    } else if ...

If the command times out, qla2x00_issue_iocb() returns a non-zero rval, but
execution continues. The code then reuses rsp_pkt to send an ABTS response
and unconditionally frees it via dma_free_coherent() right after.

If the firmware is still holding the physical DMA address and completes the
delayed command later, wouldn't it perform a DMA write to the freed physical
address?

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