Re: [PATCH v3 80/88] scsi: qla2xxx: Skip NVMe LS reject IOCB when FW not started

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Use-After-Free and list corruption on NVMe LS response error path.
- [Critical] Concurrent lockless list manipulation of `fcport->unsol_ctx_head` across interrupt and process contexts.
- [High] Lockless read-modify-write bitfield race on `ha->flags` corrupts state and causes lost interrupts.
--

--- Patch [80]: [PATCH v3 80/88] scsi: qla2xxx: Skip NVMe LS reject IOCB when FW not started ---
commit 4b34d405682300e7feb89bc5e0c5433498c1683d
Author: Nilesh Javali <njavali@marvell.com>
Date:   Tue Jul 7 05:54:27 2026 +0000

scsi: qla2xxx: Skip NVMe LS reject IOCB when FW not started

This patch modifies the error paths in qla_nvme_xmt_ls_rsp() and
qla2xxx_process_purls_pkt() to check if the firmware is started before
sending an NVMe LS reject IOCB. This avoids ringing the doorbell and queuing
IOCBs when the firmware is stopped or resetting.

> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> index fc8a344ec7d81..28a04e0ff660b 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -441,9 +441,11 @@ static int qla_nvme_xmt_ls_rsp(struct nvme_fc_local_port *lport,
>  	a.vp_idx = vha->vp_idx;
>  	a.nport_handle = uctx->nport_handle;
>  	a.xchg_address = uctx->exchange_address;
> -	spin_lock_irqsave(ha->base_qpair->qp_lock_ptr, flags);
> -	qla_nvme_ls_reject_iocb(vha, ha->base_qpair, &a, true);
> -	spin_unlock_irqrestore(ha->base_qpair->qp_lock_ptr, flags);
> +	if (ha->flags.fw_started) {

[Severity: High]
This is a pre-existing issue, but does using a bitfield for ha->flags cause
read-modify-write races?

Looking at struct qla_hw_data, fw_started and mbox_int share the same 32-bit
word:

drivers/scsi/qla2xxx/qla_def.h:
	volatile struct {
		uint32_t	mbox_int		:1;
...
		uint32_t	fw_started:1;
...
	} flags;

Since qla2x00_abort_isp_cleanup() locklessly clears fw_started in process
context, and qla2x00_mbx_completion() sets mbox_int in interrupt context,
couldn't a concurrent update cause one of the flags to be lost and stall the
firmware initialization?

> +		spin_lock_irqsave(ha->base_qpair->qp_lock_ptr, flags);
> +		qla_nvme_ls_reject_iocb(vha, ha->base_qpair, &a, true);
> +		spin_unlock_irqrestore(ha->base_qpair->qp_lock_ptr, flags);
> +	}
>  	kfree(uctx);

[Severity: Critical]
This is a pre-existing issue, but does this error path leave a dangling
pointer in the fcport->unsol_ctx_head list?

When kfree(uctx) is called here, it doesn't appear to be removed from the
list via list_del(&uctx->elem). Later unsolicited requests could then attempt
to use list_add_tail() on the corrupted list pointers of the freed block,
potentially leading to slab corruption.

>  	return rval;
>  }

[ ... ]

> @@ -1321,9 +1323,14 @@ qla2xxx_process_purls_pkt(struct scsi_qla_host *vha, struct purex_item *item)
>  		a.vp_idx = vha->vp_idx;
>  		a.nport_handle = uctx->nport_handle;
>  		a.xchg_address = uctx->exchange_address;
> -		spin_lock_irqsave(vha->hw->base_qpair->qp_lock_ptr, flags);
> -		qla_nvme_ls_reject_iocb(vha, vha->hw->base_qpair, &a, true);
> -		spin_unlock_irqrestore(vha->hw->base_qpair->qp_lock_ptr, flags);
> +		if (vha->hw->flags.fw_started) {
> +			spin_lock_irqsave(vha->hw->base_qpair->qp_lock_ptr,
> +					  flags);
> +			qla_nvme_ls_reject_iocb(vha, vha->hw->base_qpair, &a,
> +						true);
> +			spin_unlock_irqrestore(vha->hw->base_qpair->qp_lock_ptr,
> +					       flags);
> +		}
>  		list_del(&uctx->elem);

[Severity: Critical]
This is a pre-existing issue, but does this list manipulation require a 
spinlock to prevent concurrent corruption?

The list_del(&uctx->elem) here executes in a workqueue context. At the same
time, qla2xxx_process_purls_iocb() adds to this list from the hardware
response queue ISR context:

drivers/scsi/qla2xxx/qla_nvme.c:qla2xxx_process_purls_iocb()
    INIT_LIST_HEAD(&uctx->elem);
    list_add_tail(&uctx->elem, &fcport->unsol_ctx_head);

Could overlapping unsolicited requests race between the interrupt and workqueue
contexts and corrupt the prev/next pointers?

>  		kfree(uctx);
>  	}

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