Re: [PATCH v5 34/56] scsi: qla2xxx: Enhance ABTS processing for 29xx series

[sashiko-bot@kernel.org]

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

New issues:
- [High] Usage of C bitfields in the hardware-defined DMA structure `struct abts_entry_24xx_ext` breaks data layout on Big-Endian architectures.

Pre-existing issues:
- [Critical] The DMA buffer `rsp_pkt` can be freed while the hardware firmware is still actively processing it if the mailbox command times out.
--

--- Patch [34]: [PATCH v5 34/56] scsi: qla2xxx: Enhance ABTS processing for 29xx series ---
commit 64217fa79af8b429a3a92eb33321f697d422a0cf
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Enhance ABTS processing for 29xx series

This patch enhances ABTS processing for the 29xx series adapters by using
extended ABTS entry structures to handle the larger 128-byte IOCB format.
It introduces type-generic macros for logging and response construction
to avoid code duplication, branching on IS_QLA29XX() for type handling.

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 6fbcbdd208c38..8367aa005d1b5 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -72,15 +72,89 @@ static inline void display_Laser_info(scsi_qla_host_t *vha,
[ ... ]
> +#define QLA_LOG_ISSUE_ABTS_RSP(vha, rsp, dma, rval) do {		\
[ ... ]
> +	(rval) = qla2x00_issue_iocb((vha), (rsp), (dma), 0);		\
> +	if (rval) {							\
> +		ql_log(ql_log_warn, (vha), 0x028c,			\
> +		    "%s: iocb failed to execute -> %x\n",		\
> +		    __func__, (rval));					\
> +	} else if ((rsp)->comp_status) {				\
[ ... ]
> @@ -140,60 +212,27 @@ qla24xx_process_abts(struct scsi_qla_host *vha, struct purex_item *pkt)
[ ... ]
> +	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);

[Severity: High]
Does reading and writing the sof_type bitfield directly cause data corruption
on big-endian systems?

Since struct abts_entry_24xx_ext defines sof_type as a C bitfield over an
__le16 integer:

    __le16 vp_idx : 9;
    __le16 reserved_1_sof : 3;
    __le16 sof_type : 4;

Compilers on big-endian architectures will reverse the packing order of the
bits. If the hardware expects fixed little-endian bit offsets for DMA
reads/writes, wouldn't passing abts_ext->sof_type directly result in a
mangled bit sequence?

> +		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: Critical]
This is a pre-existing issue, but can this lead to kernel memory corruption
if the mailbox command times out?

In qla24xx_process_abts(), qla2x00_issue_iocb() executes a synchronous IOCB
command (called via QLA_LOG_ISSUE_ABTS_RSP). If this command times out, it
returns an error but the hardware reset is only scheduled asynchronously,
meaning the hardware might still be actively processing.

The function then unconditionally calls dma_free_coherent() on rsp_pkt. If
the hardware is delayed and finishes processing the timed-out command later,
could it DMA-write its completion status to this now-freed memory?

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