Re: [PATCH v4 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 1 potential issue(s) to consider:
- [Critical] Reading up to 124 bytes from a pointer derived from a 60-byte array triggers a FORTIFY_SOURCE runtime kernel panic on QLA29XX adapters.
--

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

This commit adds support for 128-byte response queue entries on 29xx
adapters. It updates status continuation IOCB processing paths to use
the correct entry type and data payload size based on the adapter type.

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 26722afa937c8..f668012de3d44 100644
> --- 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;

[Severity: Critical]
Will this trigger a FORTIFY_SOURCE panic on QLA29XX adapters?

The pointer data is unconditionally derived by casting to sts_cont_entry_t,
which has a 60-byte data array. The compiler tracks this 60-byte bound,
so when memcpy() copies up to 124 bytes later (from the 128-byte continuation
entry), FORTIFY_SOURCE will detect a read buffer overflow and panic the kernel.

> +			data_sz = qla_sts_cont_data_size(ha);
>  
> -			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;
>  			}
>  
> -			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;

[Severity: Critical]
Does this face the same FORTIFY_SOURCE bounds checking issue on the 60-byte
data array as __qla_copy_purex_to_buffer() above?

> +			data_sz = qla_sts_cont_data_size(ha);
[ ... ]
> @@ -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;

[Severity: Critical]
Does this face the same FORTIFY_SOURCE bounds checking issue on the 60-byte
data array as above?

> +			data_sz = qla_sts_cont_data_size(ha);
[ ... ]
> @@ -3682,7 +3701,7 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
>   * Extended sense data.
>   */
>  static void
> -qla2x00_status_cont_entry(struct rsp_que *rsp, sts_cont_entry_t *pkt)
> +qla2x00_status_cont_entry(struct rsp_que *rsp, void *pkt, u32 data_sz)
>  {
>  	uint8_t	sense_sz = 0;
>  	struct qla_hw_data *ha = rsp->hw;
> @@ -3691,6 +3710,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]
The commit message mentions that qla2x00_status_cont_entry() was changed to
accept void * so it can internally select the right structure based on adapter
type. However, doesn't this hardcoded cast to sts_cont_entry_t * defeat that
and restrict the pointer to a 60-byte array, causing the same FORTIFY_SOURCE
panic when sense_sz is larger than 60 bytes?

>  
>  	if (!sp || !GET_CMD_SENSE_LEN(sp))
>  		return;

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