Re: [PATCH v3 29/88] scsi: qla2xxx: Update handling of ELS IOCBs for 29xx series

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware-facing structures use C bitfields for little-endian 16-bit integers, breaking on big-endian architectures.
- [High] The extended ABTS response IOCB leaves the upper 64 bytes of the reused DMA buffer uninitialized, leaking stale hardware completion data.
- [High] Virtual Port index (vp_idx) >= 256 is truncated and corrupted during ABTS response processing on 29xx adapters due to an invalid struct cast.
--

--- Patch [29]: [PATCH v3 29/88] scsi: qla2xxx: Update handling of ELS IOCBs for 29xx series ---
commit 75f1549af39bd863f96218c48272faaf14654107
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Update handling of ELS IOCBs for 29xx series

Update ELS IOCB handling to support the extended 128-byte els_entry_24xx_ext
structure used by 29xx series adapters. Change the signatures of ELS issue
paths to accept a generic void pointer and introduce a helper
qla_els_set_vp_sof() for encoding vp_index and sof_type.

> diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h
> index cc613bc98e926..695a5b7fc7946 100644
> --- a/drivers/scsi/qla2xxx/qla_inline.h
> +++ b/drivers/scsi/qla2xxx/qla_inline.h
> @@ -863,3 +863,31 @@ qla_sts_fwi2_extract(struct qla_hw_data *ha, void *pkt,
[ ... ]
> +static inline void
> +qla_els_set_vp_sof(struct scsi_qla_host *vha, void *pkt, u16 vp_idx)
> +{
> +	if (IS_QLA29XX(vha->hw)) {
> +		struct els_entry_24xx_ext *ext = pkt;
> +
> +		ext->vp_index = vp_idx;
> +		ext->sof_type = ELS_EXT_EST_SOFI3;

[Severity: High]
Are C bitfields being used for a hardware-facing structure here?

If els_entry_24xx_ext uses __le16 with bitfields for vp_index and sof_type,
the memory layout will be compiler- and endian-dependent. On big-endian
architectures, the bitfields will be packed in reverse order compared to
what the little-endian hardware expects, which could corrupt the virtual
port and SOF values.

> +	} else {
> +		struct els_entry_24xx *e = pkt;
> +
> +		e->vp_index = vp_idx;
> +		e->sof_type = EST_SOFI3;
> +	}
> +}
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index c4e2e62f924ff..e2653620e80be 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -83,6 +83,8 @@ qla24xx_process_abts(struct scsi_qla_host *vha, struct purex_item *pkt)
[ ... ]
>  	/* send ABTS response */
> -	abts_rsp = (void *)rsp_els;
> +	abts_rsp = rsp_pkt;
>  	memset(abts_rsp, 0, sizeof(*abts_rsp));

[Severity: High]
Could this memset clear only 64 bytes instead of the full 128 bytes allocated
for 29xx adapters?

Since abts_rsp is a pointer to the 64-byte legacy struct abts_entry_24xx,
sizeof(*abts_rsp) is 64. If the buffer was populated during the ELS_IOCB_TYPE
command earlier in this function, could this leave stale hardware completion
data in the upper 64 bytes when the buffer is reused for the ABTS_RSP_TYPE
command?

[Severity: High]
Further down in this function, does this legacy cast corrupt the 9th bit of
the virtual port index for 29xx adapters?

    abts_rsp->vp_idx = abts->vp_idx;
    abts_rsp->sof_type = abts->sof_type & 0xf0;

Since the 29xx hardware packs these into a single 16-bit word, the 9th bit
of vp_idx resides in the lower nibble of the byte corresponding to the legacy
8-bit sof_type. Masking with 0xf0 would erase the 9th bit of vp_idx,
potentially breaking I/O aborts on virtual ports with an ID >= 256.

>  	abts_rsp->entry_type = ABTS_RSP_TYPE;
>  	abts_rsp->entry_count = 1;

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