Re: [PATCH v5 25/56] scsi: qla2xxx: Handle sts_cont_entry_ext_t for 29xx adapters

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] FORTIFY_SOURCE buffer overread panic in continuation IOCB processing

Pre-existing issues:
- [High] Remote Denial of Service (Infinite Loop) via crafted payload colliding with RESPONSE_PROCESSED
- [Medium] Missing read memory barrier after signature polling loop
--

--- Patch [25]: [PATCH v5 25/56] scsi: qla2xxx: Handle sts_cont_entry_ext_t for 29xx adapters ---
commit 5af8ad3c722085f26948d73f06875dd765494eea
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Handle sts_cont_entry_ext_t for 29xx adapters

This commit updates all status continuation IOCB processing paths to branch
on adapter type and handle 128-byte response queue entries for QLA29XX
hardware. It modifies several functions to use the correct data payload
size and structure cast to prevent truncation.

Closes: https://lore.kernel.org/oe-kbuild-all/202607142020.gqDaApes-lkp@intel.com/

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c

[ ... ]

> @@ -271,10 +276,12 @@ int __qla_copy_purex_to_buffer(struct scsi_qla_host *vha,
>  
>  	do {
>  		while ((total_bytes > 0) && (entry_count_remaining > 0)) {
> -			new_pkt = (sts_cont_entry_t *)rsp_q->ring_ptr;
> -			*pkt = new_pkt;
> +			*pkt = rsp_q->ring_ptr;
> +			data = ((sts_cont_entry_t *)*pkt)->data;
> +			data_sz = qla_sts_cont_data_size(ha);

[Severity: Critical]
Can this cast trigger a FORTIFY_SOURCE panic?

Looking at __qla_copy_purex_to_buffer() in drivers/scsi/qla2xxx/qla_isr.c,
the pointer data is initialized by casting the packet to sts_cont_entry_t,
which defines the data field as a statically known 60-byte array.

On QLA29XX adapters, data_sz is set to 124 bytes. When the subsequent
memcpy attempts to copy up to 124 bytes from this 60-byte pointer,
FORTIFY_SOURCE will detect a field-spanning read using __builtin_object_size
and panic the kernel with __read_overflow2.

Does this need to cast to the extended struct for 29xx adapters before
taking the address of the data field?

>  
> -			if (new_pkt->entry_type != STATUS_CONT_TYPE) {
> +			if (((sts_cont_entry_t *)*pkt)->entry_type !=
> +			    STATUS_CONT_TYPE) {

[ ... ]

> @@ -906,28 +915,24 @@ qla27xx_copy_multiple_pkt(struct scsi_qla_host *vha, void **pkt,
>  				continue;
>  			}

[Severity: High]
This is a pre-existing issue, but could this loop spin infinitely?

Looking at the context above this diff block in qla27xx_copy_multiple_pkt(),
the driver waits for the firmware to populate the continuation IOCB by
spinning until signature != RESPONSE_PROCESSED.

Since the signature field is at offset 60, it overlaps with the payload
data written by the firmware. If a remote endpoint sends a crafted
payload where the bytes at offset 60 happen to match the
RESPONSE_PROCESSED constant, the driver will never see the signature
change and will loop infinitely with interrupts disabled.

Is there a safer way to poll for DMA completion without risking payload
collisions?

[Severity: Medium]
This is a pre-existing issue, but is a read memory barrier missing here?

After breaking out of the cpu_relax() polling loop, the code immediately
reads the payload data from the ring buffer. On weakly-ordered
architectures, the CPU can speculatively load the payload data before
the signature validation completes.

Does this need a dma_rmb() after the signature check to ensure the CPU
processes the newly DMA'd packet instead of stale ring memory?

>  
> -			new_pkt = (sts_cont_entry_t *)rsp_q->ring_ptr;
> -			*pkt = new_pkt;
> +			*pkt = rsp_q->ring_ptr;
> +			data = ((sts_cont_entry_t *)*pkt)->data;
> +			data_sz = qla_sts_cont_data_size(ha);

[Severity: Critical]
Can this cast also trigger a FORTIFY_SOURCE panic during memcpy?

Similar to __qla_copy_purex_to_buffer(), casting to sts_cont_entry_t
bounds the data pointer to 60 bytes, causing a panic when copying
up to 124 bytes on QLA29XX hardware.

[ ... ]

> @@ -1190,10 +1206,12 @@ qla27xx_copy_fpin_pkt(struct scsi_qla_host *vha, void **pkt,
>  				continue;
>  			}
>  
> -			new_pkt = (sts_cont_entry_t *)rsp_q->ring_ptr;
> -			*pkt = new_pkt;
> +			*pkt = rsp_q->ring_ptr;
> +			data = ((sts_cont_entry_t *)*pkt)->data;
> +			data_sz = qla_sts_cont_data_size(ha);

[Severity: Critical]
Does this copy path also suffer from the FORTIFY_SOURCE panic?

The same 60-byte array limitation applies here when qla27xx_copy_fpin_pkt()
processes FPIN packets on QLA29XX adapters.

[ ... ]

> @@ -3691,6 +3711,7 @@ qla2x00_status_cont_entry(struct rsp_que *rsp, sts_cont_entry_t *pkt)
>  	struct scsi_cmnd *cp;
>  	uint32_t sense_len;
>  	uint8_t *sense_ptr;
> +	u8 *data = ((sts_cont_entry_t *)pkt)->data;

[Severity: Critical]
Can this cast trigger a FORTIFY_SOURCE panic when copying sense data?

Looking at qla2x00_status_cont_entry() in drivers/scsi/qla2xxx/qla_isr.c,
the data pointer is bounded to 60 bytes due to the sts_cont_entry_t cast.

When sense_sz exceeds 60 bytes on 29xx adapters, the subsequent memcpy
at the end of this function will panic the kernel.

Does this need to branch on adapter type to extract the data pointer safely?

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