[PATCH v2 00/13] scsi: qla2xxx: Misc. Bug fixes

[PATCH v2 00/13] scsi: qla2xxx: Misc. Bug fixes

[Nilesh Javali]

This series collects bug fixes and updates for the qla2xxx driver,
addressing crashes, hard lockups, and races observed during virtual
port (NPIV) teardown, async fabric scan, and BUS reset, along with
29xx-specific async event handling.

Changes in v2:
- Drop "scsi: qla2xxx: Yield work_lock in fabric scan path".
- Base vport delete (patch 08) returns -ENODEV instead of 0, so
  fc_vport_terminate() does not free the fc_vport when the driver did
  not tear the port down.
- vport DPC (patch 10) skips a vport when VPORT_DELETE is set.
  delete_progress is set only after disable, session deletion, and
  NVMe teardown.

Manish Rangankar (4):
  scsi: qla2xxx: Wait for session deletion before full-login LIP
  scsi: qla2xxx: Fix scan state machine race and IRQ-unsafe sp_unmap
  scsi: qla2xxx: Skip vport DPC processing during vport deletion
  scsi: qla2xxx: Add validation check for base vport index during delete

Nilesh Javali (9):
  scsi: qla2xxx: Update MAINTAINERS with new FC storage alias
  scsi: qla2xxx: Update few AENs for 29xx handling
  Revert "scsi: qla2xxx: Validate BSG request_len before reading
    vendor_cmd[]"
  scsi: qla2xxx: Fix double addition of fcport to gnl.fcport list
  scsi: qla2xxx: Validate vport before deletion
  scsi: qla2xxx: Fix use-after-free races during vport deletion
  scsi: qla2xxx: Re-acquire vport_slock and refresh tvp during vport DPC
  scsi: qla2xxx: Fix purex IOCB copy size on non-29xx adapters
  scsi: qla2xxx: Update version to 12.00.00.2610b1

 MAINTAINERS                        |  2 +-
 drivers/scsi/qla2xxx/qla_attr.c    |  6 ++++
 drivers/scsi/qla2xxx/qla_bsg.c     | 57 +++---------------------------
 drivers/scsi/qla2xxx/qla_def.h     |  1 -
 drivers/scsi/qla2xxx/qla_gs.c      | 28 ++++++++++++---
 drivers/scsi/qla2xxx/qla_init.c    | 40 ++++++++++++++-------
 drivers/scsi/qla2xxx/qla_isr.c     | 16 +++++----
 drivers/scsi/qla2xxx/qla_mid.c     | 34 +++++++++++++++---
 drivers/scsi/qla2xxx/qla_os.c      | 32 ++++++++++-------
 drivers/scsi/qla2xxx/qla_version.h |  4 +--
 10 files changed, 123 insertions(+), 97 deletions(-)

base-commit: 12e67eb89eb2b9516685c744d3f7de0a2d1bd701
-- 
2.52.0

[PATCH v2 01/13] scsi: qla2xxx: Update MAINTAINERS with new FC storage alias

[Nilesh Javali]

Update the MAINTAINERS entry for the QLA2XXX FC-SCSI driver, replacing
the GR-QLogic-Storage-Upstream@marvell.com mailing list alias with
GR-FC-Storage-Upstream@marvell.com.

Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9c768040658d..776eb78deb63 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21970,7 +21970,7 @@ F:	drivers/scsi/qla1280.[ch]
 
 QLOGIC QLA2XXX FC-SCSI DRIVER
 M:	Nilesh Javali <njavali@marvell.com>
-M:	GR-QLogic-Storage-Upstream@marvell.com
+M:	GR-FC-Storage-Upstream@marvell.com
 L:	linux-scsi@vger.kernel.org
 S:	Supported
 F:	drivers/scsi/qla2xxx/
-- 
2.52.0

[PATCH v2 02/13] scsi: qla2xxx: Update few AENs for 29xx handling

[Nilesh Javali]

Handle QLA29XX adapters like the 27xx/28xx generation across several
async event paths in qla2x00_async_event():

- MBA_SYSTEM_ERR: capture and log the extended mailbox4-7 words, and
  trigger the MPI firmware dump when mailbox7 BIT_8 is set.
- MBA_LOOP_UP: report FEC-enabled status on link up.
- MBA_IDC_AEN: process the 8200 heartbeat/IDC AEN via
  qla27xx_handle_8200_aen().
- MBA_DPORT_DIAGNOSTICS: decode and log the D-Port diagnostics result.
- MBA_TEMPERATURE_ALERT: report laser information.

29xx is FWI2-capable and uses the device_reg_24xx mailbox layout, and
qla29xx_isp_ops already wires fw_dump/mpi_fw_dump to the 27xx helpers,
so the same handling applies.

Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_isr.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 63b70d4abbf0..7bae26d4404c 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1443,7 +1443,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 		vha->hw_err_cnt++;
 
 		if (IS_QLA81XX(ha) || IS_QLA83XX(ha) ||
-		    IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
+		    IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) {
 			u16 m[4];
 
 			m[0] = rd_reg_word(&reg24->mailbox4);
@@ -1459,7 +1459,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 			    "ISP System Error - mbx1=%xh mbx2=%xh mbx3=%xh.\n ",
 			    mb[1], mb[2], mb[3]);
 
-		if ((IS_QLA27XX(ha) || IS_QLA28XX(ha)) &&
+		if ((IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) &&
 		    rd_reg_word(&reg24->mailbox7) & BIT_8)
 			ha->isp_ops->mpi_fw_dump(vha, 1);
 		ha->isp_ops->fw_dump(vha);
@@ -1554,7 +1554,8 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 		    "LOOP UP detected (%s Gbps).\n",
 		    qla2x00_get_link_speed_str(ha, ha->link_data_rate));
 
-		if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
+		if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha) ||
+		    IS_QLA29XX(ha)) {
 			if (mb[2] & BIT_0)
 				ql_log(ql_log_info, vha, 0x11a0,
 				    "FEC=enabled (link up).\n");
@@ -1998,7 +1999,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 		break;
 
 	case MBA_IDC_AEN:
-		if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
+		if (IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) {
 			vha->hw_err_cnt++;
 			qla27xx_handle_8200_aen(vha, mb);
 		} else if (IS_QLA83XX(ha)) {
@@ -2022,7 +2023,8 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 		    "D-Port Diagnostics: %04x %04x %04x %04x\n",
 		    mb[0], mb[1], mb[2], mb[3]);
 		memcpy(vha->dport_data, mb, sizeof(vha->dport_data));
-		if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
+		if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha) ||
+		    IS_QLA29XX(ha)) {
 			static char *results[] = {
 			    "start", "done(pass)", "done(error)", "undefined" };
 			static char *types[] = {
@@ -2050,7 +2052,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 		break;
 
 	case MBA_TEMPERATURE_ALERT:
-		if (IS_QLA27XX(ha) || IS_QLA28XX(ha))
+		if (IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha))
 			display_Laser_info(vha, mb[1], mb[2], mb[3]);
 		ql_dbg(ql_dbg_async, vha, 0x505e,
 		    "TEMPERATURE ALERT: %04x %04x %04x\n", mb[1], mb[2], mb[3]);
-- 
2.52.0

[PATCH v2 03/13] Revert "scsi: qla2xxx: Validate BSG request_len before reading vendor_cmd[]"

[Nilesh Javali]

The added request_len checks reject legitimate FC_BSG_HST_VENDOR requests
from the Marvell qaucli management utility. qaucli passes a request_len
that matches the historical userspace layout; it is smaller than
sizeof(struct fc_bsg_request) plus the vendor_cmd words the driver
started requiring. qla2x00_process_vendor_specific() and several
vendor sub-handlers therefore return -EINVAL ("BSG request too small
for vendor cmd") and management commands fail.

Restore the previous behavior until validation is reworked to match how
admin tools size BSG requests (for example per-command minimum lengths
or userspace fixes) without breaking supported management paths.

This reverts commit 4cf38dd9465736141263ebb63375868311a0ec81.

Cc: stable@vger.kernel.org
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_bsg.c | 57 +++-------------------------------
 1 file changed, 4 insertions(+), 53 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index ab559048dbb8..19a514a153ae 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -160,12 +160,6 @@ qla24xx_proc_fcp_prio_cfg_cmd(struct bsg_job *bsg_job)
 		goto exit_fcp_prio_cfg;
 	}
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t)) {
-		ret = -EINVAL;
-		goto exit_fcp_prio_cfg;
-	}
-
 	/* Get the sub command */
 	oper = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
 
@@ -764,10 +758,6 @@ qla2x00_process_loopback(struct bsg_job *bsg_job)
 		return -EIO;
 	}
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + 3 * sizeof(uint32_t))
-		return -EINVAL;
-
 	memset(&elreq, 0, sizeof(elreq));
 
 	elreq.req_sg_cnt = dma_map_sg(&ha->pdev->dev,
@@ -1000,10 +990,6 @@ qla84xx_reset(struct bsg_job *bsg_job)
 		return -EINVAL;
 	}
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
-		return -EINVAL;
-
 	flag = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
 
 	rval = qla84xx_reset_chip(vha, flag == A84_ISSUE_RESET_DIAG_FW);
@@ -1048,10 +1034,6 @@ qla84xx_updatefw(struct bsg_job *bsg_job)
 		return -EINVAL;
 	}
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
-		return -EINVAL;
-
 	sg_cnt = dma_map_sg(&ha->pdev->dev, bsg_job->request_payload.sg_list,
 		bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);
 	if (!sg_cnt) {
@@ -1529,15 +1511,9 @@ qla2x00_read_optrom(struct bsg_job *bsg_job)
 	struct Scsi_Host *host = fc_bsg_to_shost(bsg_job);
 	scsi_qla_host_t *vha = shost_priv(host);
 	struct qla_hw_data *ha = vha->hw;
-	uint32_t start;
+	uint32_t start = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
 	int rval = 0;
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
-		return -EINVAL;
-
-	start = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
-
 	if (ha->flags.nic_core_reset_hdlr_active)
 		return -EBUSY;
 
@@ -1580,15 +1556,9 @@ qla2x00_update_optrom(struct bsg_job *bsg_job)
 	struct Scsi_Host *host = fc_bsg_to_shost(bsg_job);
 	scsi_qla_host_t *vha = shost_priv(host);
 	struct qla_hw_data *ha = vha->hw;
-	uint32_t start;
+	uint32_t start = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
 	int rval = 0;
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
-		return -EINVAL;
-
-	start = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
-
 	mutex_lock(&ha->optrom_mutex);
 	rval = qla2x00_optrom_setup(bsg_job, vha, start, 1);
 	if (rval) {
@@ -2453,11 +2423,6 @@ qlafx00_mgmt_cmd(struct bsg_job *bsg_job)
 	struct fc_port *fcport;
 	char  *type = "FC_BSG_HST_FX_MGMT";
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + sizeof(uint32_t) +
-	    sizeof(struct qla_mt_iocb_rqst_fx00))
-		return -EINVAL;
-
 	/* Copy the IOCB specific information */
 	piocb_rqst = (struct qla_mt_iocb_rqst_fx00 *)
 	    &bsg_request->rqst_data.h_vendor.vendor_cmd[1];
@@ -3379,13 +3344,6 @@ qla2x00_process_vendor_specific(struct scsi_qla_host *vha, struct bsg_job *bsg_j
 {
 	struct fc_bsg_request *bsg_request = bsg_job->request;
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + sizeof(uint32_t)) {
-		ql_log(ql_log_warn, vha, 0x7000,
-		       "BSG request too small for vendor cmd.\n");
-		return -EINVAL;
-	}
-
 	ql_dbg(ql_dbg_edif, vha, 0x911b, "%s FC_BSG_HST_VENDOR cmd[0]=0x%x\n",
 	    __func__, bsg_request->rqst_data.h_vendor.vendor_cmd[0]);
 
@@ -3529,11 +3487,8 @@ qla24xx_bsg_request(struct bsg_job *bsg_job)
 	}
 
 	/* Disable port will bring down the chip, allow enable command */
-	if (bsg_request->msgcode == FC_BSG_HST_VENDOR &&
-	    bsg_job->request_len >=
-		sizeof(struct fc_bsg_request) + sizeof(uint32_t) &&
-	    (bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_MANAGE_HOST_PORT ||
-	     bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_GET_HOST_STATS))
+	if (bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_MANAGE_HOST_PORT ||
+	    bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_GET_HOST_STATS)
 		goto skip_chip_chk;
 
 	if (vha->hw->flags.port_isolated) {
@@ -3842,10 +3797,6 @@ static int qla28xx_validate_flash_image(struct bsg_job *bsg_job)
 	if (!IS_QLA28XX(ha) || vha->vp_idx != 0)
 		return -EPERM;
 
-	if (bsg_job->request_len <
-	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
-		return -EINVAL;
-
 	mutex_lock(&ha->optrom_mutex);
 	rval = qla28xx_do_validate_flash_image(bsg_job, &state);
 	if (rval)
-- 
2.52.0

[PATCH v2 04/13] scsi: qla2xxx: Wait for session deletion before full-login LIP

[Nilesh Javali]

From: Manish Rangankar <mrangankar@marvell.com>

In qla2x00_loop_reset(), the LIP_FULL_LOGIN path called
qla2x00_mark_all_devices_lost() and immediately fired the firmware
full-login mailbox. In N2N topology, when the local port has the
larger WWPN, an fcport can still be in DELETE_PEND and not yet pruned
from the list when the FW LIP completes and RIDA processing kicks in,
leaving the driver to act on a stale fcport instance.

Replace mark_all_devices_lost() with qla2x00_wait_for_sess_deletion(),
which marks the fcports lost, waits for fcport_count to drain, and
flushes the host workqueue. That guarantees fcport workers have run to
completion and the stale entries are gone before FW is asked to
re-LIP, eliminating the race during BUS reset.

Cc: stable@vger.kernel.org
Fixes: 3c75ad1d87c7 ("scsi: qla2xxx: Remove defer flag to indicate immeadiate port loss")
Signed-off-by: Manish Rangankar <mrangankar@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_os.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index c0efdbff5da7..0e5e672e81ee 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1752,7 +1752,7 @@ qla2x00_loop_reset(scsi_qla_host_t *vha)
 	if (ha->flags.enable_lip_full_login && !IS_CNA_CAPABLE(ha)) {
 		atomic_set(&vha->loop_state, LOOP_DOWN);
 		atomic_set(&vha->loop_down_timer, LOOP_DOWN_TIME);
-		qla2x00_mark_all_devices_lost(vha);
+		qla2x00_wait_for_sess_deletion(vha);
 		ret = qla2x00_full_login_lip(vha);
 		if (ret != QLA_SUCCESS) {
 			ql_dbg(ql_dbg_taskm, vha, 0x802d,
-- 
2.52.0

[PATCH v2 05/13] scsi: qla2xxx: Fix double addition of fcport to gnl.fcport list

[Nilesh Javali]

The lockless FCF_ASYNC_SENT check in qla24xx_async_gnl() is only a
fast-path optimization. When two work items for the same fcport race,
both can pass the lockless check before either takes sess_lock, and the
second thread then calls list_add_tail() on an already-listed gnl_entry,
tripping __list_add_valid_or_report() and crashing the kernel.

list_add double add: new=ff47fbf2d9b34158, prev=ff47fbf2d9b34158,
next=ff47fbf28788bca8.
[ cut here ]
BUG: kernel BUG at lib/list_debug.c:35!
Oops: invalid opcode: 0000 [#1] SMP NOPTI
qla2xxx [0000:a2:00.1]-8037:17: F/W Ready - OK.
Workqueue: qla2xxx_wq qla2x00_iocb_work_fn [qla2xxx]
RIP: 0010:__list_add_valid_or_report.cold+0x23/0x5b
RSP: 0018:ff7e6f67487c3d30 EFLAGS: 00010046
RAX: 0000000000000058 RBX: ff47fbf28788bca8 RCX: 0000000000000027
RDX: 0000000000000000 RSI: 0000000000000001 RDI: ff47fbfa5f19ccc0
RBP: ff47fbf2d9b34034 R08: 0000000000000000 R09: ffffffffbcd45c98
R10: ffffffffbcae35d8 R11: 0000000000000003 R12: ff47fbf2d9b34000
R13: ff47fbf28788b838 R14: ff47fbf2d9b34158 R15: ff47fbf2d9b34158
FS:  0000000000000000(0000) GS:ff47fbfa5f180000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fa5ea7bade8 CR3: 000000102b422005 CR4: 0000000000773ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
  <TASK>
  ? show_trace_log_lvl+0x1b0/0x2f0
  ? show_trace_log_lvl+0x1b0/0x2f0
  ? qla2xxx_async_gnl+0x136/0x300 [qla2xxx]
  qla2xxx [0000:a2:00.1]-803a:17: fw_state=3 (7, 8144, 0, 0 0) curr time=102ddc336.

Drop FCF_ASYNC_SENT from the lockless guard and re-check it under
sess_lock so the test-and-set of FCF_ASYNC_SENT is atomic with the
list_add_tail().

Cc: stable@vger.kernel.org
Fixes: 3dbec59bdf63 ("scsi: qla2xxx: Prevent multiple active discovery commands per session")
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index bb0cc71de37b..515a7ea4299d 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -1158,13 +1158,17 @@ int qla24xx_async_gnl(struct scsi_qla_host *vha, fc_port_t *fcport)
 	unsigned long flags;
 	u16 *mb;
 
-	if (!vha->flags.online || (fcport->flags & FCF_ASYNC_SENT))
+	if (!vha->flags.online)
 		goto done;
 
 	ql_dbg(ql_dbg_disc, vha, 0x20d9,
 	    "Async-gnlist WWPN %8phC \n", fcport->port_name);
 
 	spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
+	if (fcport->flags & FCF_ASYNC_SENT) {
+		spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
+		return QLA_SUCCESS;
+	}
 	fcport->flags |= FCF_ASYNC_SENT;
 	qla2x00_set_fcport_disc_state(fcport, DSC_GNL);
 	fcport->last_rscn_gen = fcport->rscn_gen;
-- 
2.52.0

[PATCH v2 06/13] scsi: qla2xxx: Fix scan state machine race and IRQ-unsafe sp_unmap

[Nilesh Javali]

From: Manish Rangankar <mrangankar@marvell.com>

qla_async_scan_sp_done() runs in hard IRQ context. Fix three issues:

1. scan.step race: the handler cleared SF_SCANNING under work_lock but
   read vha->scan.step outside the lock. Another CPU could start a fresh
   scan and overwrite scan.step to FAB_SCAN_START before the read, which
   is not handled in the switch and trips WARN_ON(1) via the default
   case. Capture scan.step into a local under the same work_lock hold
   that clears SF_SCANNING and switch on the saved copy.

2. IRQ-unsafe unmap (error path): when post of QLA_EVT_SCAN_FINISH
   failed, qla24xx_sp_unmap() was called directly in IRQ context. It
   does dma_free_coherent()/vunmap() which require process context.
   Defer the unmap via the QLA_EVT_UNMAP work event instead.

3. IRQ-unsafe unmap (success-path fallback): same direct sp_unmap in
   IRQ context when post_next_scan_work() failed or the switch hit the
   default case. Defer via QLA_EVT_UNMAP as well.

Cc: stable@vger.kernel.org
Fixes: beafd6924614 ("scsi: qla2xxx: Reduce fabric scan duplicate code")
Signed-off-by: Manish Rangankar <mrangankar@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_gs.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index 20b1aef455c4..efe98609c149 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -3512,6 +3512,8 @@ static void qla2x00_find_free_fcp_nvme_slot(struct scsi_qla_host *vha,
 static void qla_async_scan_sp_done(srb_t *sp, int res)
 {
 	struct scsi_qla_host *vha = sp->vha;
+	struct qla_work_evt *e;
+	enum scan_step step;
 	unsigned long flags;
 	int rc;
 
@@ -3544,8 +3546,16 @@ static void qla_async_scan_sp_done(srb_t *sp, int res)
 		 */
 		rc = qla2x00_post_next_scan_work(vha, sp, QLA_EVT_SCAN_FINISH);
 		if (rc) {
-			/* Cleanup here to prevent memory leak */
-			qla24xx_sp_unmap(vha, sp);
+			/* Defer sp_unmap to process context -- we are in IRQ */
+			e = qla2x00_alloc_work(vha, QLA_EVT_UNMAP);
+			if (e) {
+				e->u.iosb.sp = sp;
+				qla2x00_post_work(vha, e);
+			} else {
+				ql_log(ql_log_warn, vha, 0x2029,
+				       "%s: Failed to schedule sp_unmap, sp leaked\n",
+				       name);
+			}
 
 			spin_lock_irqsave(&vha->work_lock, flags);
 			vha->scan.scan_flags &= ~SF_SCANNING;
@@ -3568,10 +3578,11 @@ static void qla_async_scan_sp_done(srb_t *sp, int res)
 	qla2x00_find_free_fcp_nvme_slot(vha, sp);
 
 	spin_lock_irqsave(&vha->work_lock, flags);
+	step = vha->scan.step;
 	vha->scan.scan_flags &= ~SF_SCANNING;
 	spin_unlock_irqrestore(&vha->work_lock, flags);
 
-	switch (vha->scan.step) {
+	switch (step) {
 	case FAB_SCAN_GPNFT_FCP:
 	case FAB_SCAN_GPNFT_NVME:
 		rc = qla2x00_post_next_scan_work(vha, sp, QLA_EVT_SCAN_CMD);
@@ -3594,7 +3605,16 @@ static void qla_async_scan_sp_done(srb_t *sp, int res)
 	}
 
 	if (rc) {
-		qla24xx_sp_unmap(vha, sp);
+		/* Defer sp_unmap to process context -- we are in IRQ */
+		e = qla2x00_alloc_work(vha, QLA_EVT_UNMAP);
+		if (e) {
+			e->u.iosb.sp = sp;
+			qla2x00_post_work(vha, e);
+		} else {
+			ql_log(ql_log_warn, vha, 0x202a,
+			       "%s: Failed to schedule sp_unmap, sp leaked\n",
+			       __func__);
+		}
 		set_bit(LOCAL_LOOP_UPDATE, &vha->dpc_flags);
 		set_bit(LOOP_RESYNC_NEEDED, &vha->dpc_flags);
 		return;
-- 
2.52.0

[PATCH v2 07/13] scsi: qla2xxx: Skip vport DPC processing during vport deletion

[Nilesh Javali]

From: Manish Rangankar <mrangankar@marvell.com>

A race condition exists between qla2x00_do_dpc_all_vps(), which processes
virtual ports via ha->vp_list, and a user-triggered vport deletion (e.g.,
fc_vport_terminate). During teardown, vport delete may free resources used
by the DPC thread, including workqueues and fabric-related structures. If
the DPC thread is concurrently accessing these, a NULL pointer dereference
or use-after-free can occur.

Skip DPC processing for vports that are in the process of being deleted by
checking the vha->flags.delete_progress flag.

Cc: stable@vger.kernel.org
Fixes: feafb7b1714c ("[SCSI] qla2xxx: Fix vport delete issues")
Signed-off-by: Manish Rangankar <mrangankar@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_mid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index 4ad23d206add..f7a08bc0093a 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -439,7 +439,7 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
 
 	spin_lock_irqsave(&ha->vport_slock, flags);
 	list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
-		if (vp->vp_idx) {
+		if (vp->vp_idx && !vha->flags.delete_progress) {
 			atomic_inc(&vp->vref_count);
 			spin_unlock_irqrestore(&ha->vport_slock, flags);
 
-- 
2.52.0

[PATCH v2 08/13] scsi: qla2xxx: Add validation check for base vport index during delete

[Nilesh Javali]

From: Manish Rangankar <mrangankar@marvell.com>

Add a guard in qla24xx_vport_delete() to prevent deleting a vport with an
invalid or zero vp_idx. If vp_idx is zero, log a fatal message and return
-ENODEV instead of proceeding with deletion. fc_vport_terminate() treats
0 as success and would free the fc_vport without tearing the scsi host
down.

    [ 4268.547235] Call Trace:
    [ 4268.547238]  <TASK>
    [ 4268.547242]  ? srso_alias_return_thunk+0x5/0xfbef5
    [ 4268.547256]  ? show_trace_log_lvl+0x26e/0x2df
    [ 4268.547270]  ? show_trace_log_lvl+0x26e/0x2df
    [ 4268.547279]  ? qla2x00_do_dpc_all_vps+0xc2/0xf0 [qla2xxx]
    [ 4268.547305]  ? __die_body.cold+0x8/0xd
    [ 4268.547310]  ? die_addr+0x39/0x60
    [ 4268.547322]  ? exc_general_protection+0x1ec/0x420
    [ 4268.547336]  ? asm_exc_general_protection+0x22/0x30
    [ 4268.547347]  ? qla2x00_do_dpc_all_vps+0xc2/0xf0 [qla2xxx]
    [ 4268.547369]  ? qla2x00_do_dpc+0x264/0x760 [qla2xxx]
    [ 4268.547391]  ? qla2xxx_pci_error_detected+0x190/0x190 [qla2xxx]
    [ 4268.547411]  ? kthread+0xe0/0x100
    [ 4268.547420]  ? __pfx_kthread+0x10/0x10
    [ 4268.547427]  ? ret_from_fork+0x2c/0x50
    [ 4268.547442]  </TASK>

Fixes: feafb7b1714c ("[SCSI] qla2xxx: Fix vport delete issues")
Cc: stable@vger.kernel.org
Signed-off-by: Manish Rangankar <mrangankar@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_attr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index a4ca22024ede..15fcfd2246e4 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -3276,6 +3276,12 @@ qla24xx_vport_delete(struct fc_vport *fc_vport)
 	struct qla_hw_data *ha = vha->hw;
 	uint16_t id = vha->vp_idx;
 
+	if (!vha->vp_idx) {
+		ql_log(ql_log_fatal, vha, 0x700f,
+		       "failed to delete Vport idx [%d]\n", vha->vp_idx);
+		return -ENODEV;
+	}
+
 	set_bit(VPORT_DELETE, &vha->dpc_flags);
 
 	while (test_bit(LOOP_RESYNC_ACTIVE, &vha->dpc_flags))
-- 
2.52.0

[PATCH v2 09/13] scsi: qla2xxx: Validate vport before deletion

[Nilesh Javali]

While the vport_slock is dropped, another thread can delete and free the
vport. After re-acquiring the lock, accessing vp->vref_count on a freed
vport leads to a use-after-free and a system crash (general protection
fault in the DPC path).

     general protection fault, probably for non-canonical address 0xdead000000000100: 0000 [#1] SMP NOPTI
     qla2xxx [0000:27:00.1]-2026:80: Async done-gnnft res 0 step 2
     RIP: 0010:qla2x00_do_dpc_all_vps+0x2e1/0x4f0 [qla2xxx]
     RSP: 0018:ffff9e284717fea0 EFLAGS: 00010046
     qla2xxx [0000:27:00.1]-2003:80: qla_fab_async_scan: step 3, rsp size 32784, req size 20 hdl c3 gpnft FC4TYPE 28

     RAX: 0000000000000246 RBX: ffff8a602408ca58 RCX: ffffffffc0ce40d3
     RDX: dead000000000100 RSI: ffff8a679b80ce40 RDI: ffff8a606bff0488
     RBP: ffff8a606bff07c0 R08: ffffffffc0cdc8f0 R09: ffff8a679b80cc68
     R10: ffff8a678643a898 R11: ffff9e284717fbb8 R12: ffff8a606bff0488
     R13: dead000000000100 R14: dead000000000100 R15: ffff8a606bff0000
     FS:  0000000000000000(0000) GS:ffff8a6f5f540000(0000) knlGS:0000000000000000
     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
     CR2: 00007f6ebf576974 CR3: 0000000920ab4000 CR4: 0000000000350ee0
     Call Trace:
      ? qla2x00_do_dpc+0x2bc/0x750 [qla2xxx]
      ? qla2xxx_pci_error_detected+0x130/0x130 [qla2xxx]
      ? kthread+0x134/0x150
      ? set_kthread_struct+0x50/0x50
      ? ret_from_fork+0x35/0x40

Validate the vport (vp_idx and delete_progress flag) before decrementing
vref_count in qla2x00_abort_isp_cleanup(), qla2x00_do_dpc_all_vps() and
qla_pci_error_cleanup().

Cc: stable@vger.kernel.org
Fixes: feafb7b1714c ("[SCSI] qla2xxx: Fix vport delete issues")
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 4 +++-
 drivers/scsi/qla2xxx/qla_mid.c  | 4 +++-
 drivers/scsi/qla2xxx/qla_os.c   | 8 ++++++--
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 515a7ea4299d..7898521b76d4 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -7579,7 +7579,9 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha)
 			qla2x00_mark_all_devices_lost(vp);
 
 			spin_lock_irqsave(&ha->vport_slock, flags);
-			atomic_dec(&vp->vref_count);
+			/* Check if vp is still valid before accessing it */
+			if (vp->vp_idx && !vp->flags.delete_progress)
+				atomic_dec(&vp->vref_count);
 		}
 		spin_unlock_irqrestore(&ha->vport_slock, flags);
 	} else {
diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index f7a08bc0093a..90f09d36705a 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -446,7 +446,9 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
 			qla2x00_do_dpc_vp(vp);
 
 			spin_lock_irqsave(&ha->vport_slock, flags);
-			atomic_dec(&vp->vref_count);
+			/* Check if vp is still valid before accessing it */
+			if (!vp->flags.delete_progress)
+				atomic_dec(&vp->vref_count);
 		}
 	}
 	spin_unlock_irqrestore(&ha->vport_slock, flags);
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 0e5e672e81ee..0047c2bd531b 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -7945,7 +7945,9 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha)
 		spin_unlock_irqrestore(&ha->vport_slock, flags);
 		qla2x00_mark_all_devices_lost(vp);
 		spin_lock_irqsave(&ha->vport_slock, flags);
-		atomic_dec(&vp->vref_count);
+		/* Check if vp is still valid before accessing it */
+		if (vp->vp_idx && !vp->flags.delete_progress)
+			atomic_dec(&vp->vref_count);
 	}
 	spin_unlock_irqrestore(&ha->vport_slock, flags);
 
@@ -7960,7 +7962,9 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha)
 		list_for_each_entry(fcport, &vp->vp_fcports, list)
 			fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT);
 		spin_lock_irqsave(&ha->vport_slock, flags);
-		atomic_dec(&vp->vref_count);
+		/* Check if vp is still valid before accessing it */
+		if (vp->vp_idx && !vp->flags.delete_progress)
+			atomic_dec(&vp->vref_count);
 	}
 	spin_unlock_irqrestore(&ha->vport_slock, flags);
 }
-- 
2.52.0

[PATCH v2 10/13] scsi: qla2xxx: Fix use-after-free races during vport deletion

[Nilesh Javali]

System continued to crash during vport deletion due to use-after-free
races on the vport list. While the vport_slock is dropped, another thread
can delete and free the vport; subsequently touching vp->vref_count on the
freed node leads to a general protection fault.

 general protection fault, probably for non-canonical address 0xdead000000000100: 0000 [#1] SMP NOPTI
 RIP: 0010:qla2x00_do_dpc_all_vps+0x8d/0x4f0 [qla2xxx]
 RSP: 0018:ffff9c4e07047ea0 EFLAGS: 00010002
 RAX: 0000000000000246 RBX: ffff8da4d7a7aa58 RCX: 0000000000000027
 RDX: dead000000000100 RSI: 000000010d2b276c RDI: ffff8da4d01d4488
 RBP: ffff8da4d01d47c0 R08: 0000000000000000 R09: c00000010d2b276c
  R10: ffff8da614945098 R11: ffff9c4e07047bb8 R12: ffff8da4d01d4488
  R13: dead000000000100 R14: dead000000000100 R15: ffff8da4d01d4000
  FS:  0000000000000000(0000) GS:ffff8dac1f0c0000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 00007f9e55261974 CR3: 0000000a46410000 CR4: 0000000000350ee0
  Call Trace:
   ? qla2x00_do_dpc+0x2bc/0x750 [qla2xxx]
   ? qla2xxx_pci_error_detected+0x130/0x130 [qla2xxx]
   ? kthread+0x134/0x150
   ? set_kthread_struct+0x50/0x50
   ? ret_from_fork+0x35/0x40

Validate the vport (vp_idx and delete_progress flag) before referencing it
across all the remaining vp_list traversals in qla2x00_abort_isp_cleanup(),
qla2x00_abort_isp(), qla2x00_alert_all_vps() and qla_pci_error_cleanup().
Also use list_del_init() instead of list_del() in
qla24xx_deallocate_vp_id() so a concurrent traversal does not crash on
the poisoned list node value.

DPC skips the vport when VPORT_DELETE is set, because delete_progress is
not set until teardown is already underway.

Cc: stable@vger.kernel.org
Fixes: feafb7b1714c ("[SCSI] qla2xxx: Fix vport delete issues")
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 34 ++++++++++++++++++++-------------
 drivers/scsi/qla2xxx/qla_mid.c  | 10 ++++++----
 drivers/scsi/qla2xxx/qla_os.c   | 30 ++++++++++++++++-------------
 3 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 7898521b76d4..c02571a6f0c7 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -7573,15 +7573,17 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha)
 
 		spin_lock_irqsave(&ha->vport_slock, flags);
 		list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
-			atomic_inc(&vp->vref_count);
-			spin_unlock_irqrestore(&ha->vport_slock, flags);
+			if (vp->vp_idx && !vp->flags.delete_progress) {
+				atomic_inc(&vp->vref_count);
+				spin_unlock_irqrestore(&ha->vport_slock, flags);
 
-			qla2x00_mark_all_devices_lost(vp);
+				qla2x00_mark_all_devices_lost(vp);
 
-			spin_lock_irqsave(&ha->vport_slock, flags);
-			/* Check if vp is still valid before accessing it */
-			if (vp->vp_idx && !vp->flags.delete_progress)
+				spin_lock_irqsave(&ha->vport_slock, flags);
+				/* Reference held under vport_slock keeps vp valid. */
 				atomic_dec(&vp->vref_count);
+				wake_up(&vp->vref_waitq);
+			}
 		}
 		spin_unlock_irqrestore(&ha->vport_slock, flags);
 	} else {
@@ -7597,14 +7599,18 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha)
 	}
 	spin_lock_irqsave(&ha->vport_slock, flags);
 	list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
-		atomic_inc(&vp->vref_count);
-		spin_unlock_irqrestore(&ha->vport_slock, flags);
+		if (vp->vp_idx && !vp->flags.delete_progress) {
+			atomic_inc(&vp->vref_count);
+			spin_unlock_irqrestore(&ha->vport_slock, flags);
 
-		list_for_each_entry(fcport, &vp->vp_fcports, list)
-			fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT);
+			list_for_each_entry(fcport, &vp->vp_fcports, list)
+				fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT);
 
-		spin_lock_irqsave(&ha->vport_slock, flags);
-		atomic_dec(&vp->vref_count);
+			spin_lock_irqsave(&ha->vport_slock, flags);
+			/* Reference held under vport_slock keeps vp valid. */
+			atomic_dec(&vp->vref_count);
+			wake_up(&vp->vref_waitq);
+		}
 	}
 	spin_unlock_irqrestore(&ha->vport_slock, flags);
 
@@ -7786,7 +7792,7 @@ qla2x00_abort_isp(scsi_qla_host_t *vha)
 		qla2x00_configure_hba(vha);
 		spin_lock_irqsave(&ha->vport_slock, flags);
 		list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
-			if (vp->vp_idx) {
+			if (vp->vp_idx && !vp->flags.delete_progress) {
 				atomic_inc(&vp->vref_count);
 				spin_unlock_irqrestore(&ha->vport_slock, flags);
 
@@ -7801,7 +7807,9 @@ qla2x00_abort_isp(scsi_qla_host_t *vha)
 				qla2x00_vp_abort_isp(vp);
 
 				spin_lock_irqsave(&ha->vport_slock, flags);
+				/* Reference held under vport_slock keeps vp valid. */
 				atomic_dec(&vp->vref_count);
+				wake_up(&vp->vref_waitq);
 			}
 		}
 		spin_unlock_irqrestore(&ha->vport_slock, flags);
diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index 90f09d36705a..754c806dac80 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -79,7 +79,7 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha)
 	for (i = 0; i < 500; i++) {
 		spin_lock_irqsave(&ha->vport_slock, flags);
 		if (atomic_read(&vha->vref_count) == 0) {
-			list_del(&vha->list);
+			list_del_init(&vha->list);
 			qla_update_vp_map(vha, RESET_VP_IDX);
 			bailout = 1;
 		}
@@ -94,7 +94,7 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha)
 		ql_log(ql_log_info, vha, 0xfffa,
 			"vha->vref_count=%u timeout\n", vha->vref_count.counter);
 		spin_lock_irqsave(&ha->vport_slock, flags);
-		list_del(&vha->list);
+		list_del_init(&vha->list);
 		qla_update_vp_map(vha, RESET_VP_IDX);
 		spin_unlock_irqrestore(&ha->vport_slock, flags);
 	}
@@ -312,6 +312,7 @@ qla2x00_alert_all_vps(struct rsp_que *rsp, uint16_t *mb)
 			}
 
 			spin_lock_irqsave(&ha->vport_slock, flags);
+			/* Reference held under vport_slock keeps vha valid. */
 			atomic_dec(&vha->vref_count);
 			wake_up(&vha->vref_waitq);
 		}
@@ -439,7 +440,8 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
 
 	spin_lock_irqsave(&ha->vport_slock, flags);
 	list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
-		if (vp->vp_idx && !vha->flags.delete_progress) {
+		if (vp->vp_idx &&
+		    !test_bit(VPORT_DELETE, &vp->dpc_flags)) {
 			atomic_inc(&vp->vref_count);
 			spin_unlock_irqrestore(&ha->vport_slock, flags);
 
@@ -447,7 +449,7 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
 
 			spin_lock_irqsave(&ha->vport_slock, flags);
 			/* Check if vp is still valid before accessing it */
-			if (!vp->flags.delete_progress)
+			if (vp->vp_idx && !vp->flags.delete_progress)
 				atomic_dec(&vp->vref_count);
 		}
 	}
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 0047c2bd531b..5593dff7eb9b 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -7941,13 +7941,15 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha)
 
 	spin_lock_irqsave(&ha->vport_slock, flags);
 	list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
-		atomic_inc(&vp->vref_count);
-		spin_unlock_irqrestore(&ha->vport_slock, flags);
-		qla2x00_mark_all_devices_lost(vp);
-		spin_lock_irqsave(&ha->vport_slock, flags);
-		/* Check if vp is still valid before accessing it */
-		if (vp->vp_idx && !vp->flags.delete_progress)
+		if (vp->vp_idx && !vp->flags.delete_progress) {
+			atomic_inc(&vp->vref_count);
+			spin_unlock_irqrestore(&ha->vport_slock, flags);
+			qla2x00_mark_all_devices_lost(vp);
+			spin_lock_irqsave(&ha->vport_slock, flags);
+			/* Reference held under vport_slock keeps vp valid. */
 			atomic_dec(&vp->vref_count);
+			wake_up(&vp->vref_waitq);
+		}
 	}
 	spin_unlock_irqrestore(&ha->vport_slock, flags);
 
@@ -7957,14 +7959,16 @@ static void qla_pci_error_cleanup(scsi_qla_host_t *vha)
 
 	spin_lock_irqsave(&ha->vport_slock, flags);
 	list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
-		atomic_inc(&vp->vref_count);
-		spin_unlock_irqrestore(&ha->vport_slock, flags);
-		list_for_each_entry(fcport, &vp->vp_fcports, list)
-			fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT);
-		spin_lock_irqsave(&ha->vport_slock, flags);
-		/* Check if vp is still valid before accessing it */
-		if (vp->vp_idx && !vp->flags.delete_progress)
+		if (vp->vp_idx && !vp->flags.delete_progress) {
+			atomic_inc(&vp->vref_count);
+			spin_unlock_irqrestore(&ha->vport_slock, flags);
+			list_for_each_entry(fcport, &vp->vp_fcports, list)
+				fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT);
+			spin_lock_irqsave(&ha->vport_slock, flags);
+			/* Reference held under vport_slock keeps vp valid. */
 			atomic_dec(&vp->vref_count);
+			wake_up(&vp->vref_waitq);
+		}
 	}
 	spin_unlock_irqrestore(&ha->vport_slock, flags);
 }
-- 
2.52.0

[PATCH v2 11/13] scsi: qla2xxx: Re-acquire vport_slock and refresh tvp during vport DPC

[Nilesh Javali]

A hard lockup was observed after multiple iterations of vport delete. While
the vport_slock is dropped during DPC processing, the next entry (tvp) may
be removed via list_del_init(), making its list.next point to itself.
list_for_each_entry_safe then loops forever since the self-referencing node
never matches the list head.

 NMI watchdog: Watchdog detected hard LOCKUP on cpu 13
 RIP: 0010:qla2x00_do_dpc_all_vps+0x8f/0x510 [qla2xxx]
 RSP: 0018:ffffac4ac72b7ea0 EFLAGS: 00000006
 RAX: ffff95be03c12a58 RBX: ffff95be03c12a58 RCX: ffffffffc0d660ff
 RDX: 0000000000000001 RSI: ffff95be03c10a58 RDI: ffff95b688b90488
 RBP: ffff95be03c12a58 R08: ffffffffc0d5e8f0 R09: c0000001001400be
 R10: ffff95b7b580d098 R11: ffffac4ac72b7bb8 R12: ffff95b688b907c0
 R13: ffff95b688b90488 R14: 0000000000000246 R15: 0000000000000246
 FS:  0000000000000000(0000) GS:ffff95bdded40000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 000055c11dd3a730 CR3: 0000000fe1610000 CR4: 0000000000350ee0
 Call Trace:
  qla2x00_do_dpc+0x2bc/0x750 [qla2xxx]
  ? qla2xxx_pci_error_detected+0x130/0x130 [qla2xxx]
  kthread+0x134/0x150

Re-acquire vport_slock using a spin_trylock + cond_resched() yield loop to
avoid a hard lockup if the lock holder is delayed, and refresh tvp from the
current list state after re-locking (break out if vp itself was removed).
Apply the same tvp refresh in qla2x00_alert_all_vps() and add a
cond_resched() before qla2x00_loop_resync() in qla2x00_do_dpc_vp().

Cc: stable@vger.kernel.org
Fixes: feafb7b1714c ("[SCSI] qla2xxx: Fix vport delete issues")
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_mid.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index 754c806dac80..65dcfc1b04ef 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -315,6 +315,10 @@ qla2x00_alert_all_vps(struct rsp_que *rsp, uint16_t *mb)
 			/* Reference held under vport_slock keeps vha valid. */
 			atomic_dec(&vha->vref_count);
 			wake_up(&vha->vref_waitq);
+			/* refresh tvp from the current list state */
+			if (list_empty(&vha->list))
+				break;
+			tvp = list_next_entry(vha, list);
 		}
 		i++;
 	}
@@ -409,6 +413,7 @@ qla2x00_do_dpc_vp(scsi_qla_host_t *vha)
 		if (!(test_and_set_bit(LOOP_RESYNC_ACTIVE, &vha->dpc_flags))) {
 			ql_dbg(ql_dbg_dpc, vha, 0x401a,
 			    "Loop resync scheduled.\n");
+			cond_resched();
 			qla2x00_loop_resync(vha);
 			clear_bit(LOOP_RESYNC_ACTIVE, &vha->dpc_flags);
 			ql_dbg(ql_dbg_dpc, vha, 0x401b,
@@ -446,11 +451,28 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
 			spin_unlock_irqrestore(&ha->vport_slock, flags);
 
 			qla2x00_do_dpc_vp(vp);
-
-			spin_lock_irqsave(&ha->vport_slock, flags);
-			/* Check if vp is still valid before accessing it */
-			if (vp->vp_idx && !vp->flags.delete_progress)
-				atomic_dec(&vp->vref_count);
+			cond_resched();
+
+			/* Re-acquire lock with yield to avoid hard lockup if holder is delayed */
+			while (!spin_trylock_irqsave(&ha->vport_slock, flags))
+				cond_resched();
+			/* Reference held under vport_slock keeps vp valid. */
+			atomic_dec(&vp->vref_count);
+			wake_up(&vp->vref_waitq);
+			/*
+			 * While the lock was dropped, the next entry (tvp) may
+			 * have been removed via list_del_init(), making its
+			 * list.next point to itself. list_for_each_entry_safe
+			 * would then loop infinitely since the self-referencing
+			 * pointer never matches the list head.
+			 *
+			 * If vp itself was removed (deallocate timeout), we
+			 * cannot continue iterating from it, break out.
+			 * Otherwise refresh tvp from the current list state.
+			 */
+			if (list_empty(&vp->list))
+				break;
+			tvp = list_next_entry(vp, list);
 		}
 	}
 	spin_unlock_irqrestore(&ha->vport_slock, flags);
-- 
2.52.0

[PATCH v2 12/13] scsi: qla2xxx: Fix purex IOCB copy size on non-29xx adapters

[Nilesh Javali]

qla24xx_copy_std_pkt() copies a single purex IOCB from the request/
response ring into a purex_item. The source entry size depends on the
adapter: 64 bytes (struct purex_entry_24xx) on 27xx/28xx and 128 bytes
(struct purex_entry_24xx_ext) on 29xx.

For non-29xx adapters the copy size was set to QLA_DEFAULT_PAYLOAD_SIZE
(128), causing memcpy() to read 64 bytes past the end of the 64-byte
source IOCB in the ring. Copy sizeof(struct purex_entry_24xx) instead so
exactly one IOCB is copied on these adapters.

Cc: stable@vger.kernel.org
Fixes: 150a29ebda52 ("scsi: qla2xxx: Enhance purex_entry handling for 29xx series")
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_def.h | 1 -
 drivers/scsi/qla2xxx/qla_isr.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 97e2a1a9ce3f..da5ca604d99e 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -5005,7 +5005,6 @@ struct active_regions {
 #define QLA_SET_DATA_RATE_NOLR	1
 #define QLA_SET_DATA_RATE_LR	2 /* Set speed and initiate LR */
 
-#define QLA_DEFAULT_PAYLOAD_SIZE	64
 #define QLA_MAX_IOCB_SIZE		128
 /*
  * This item might be allocated with a size > sizeof(struct purex_item).
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 7bae26d4404c..2a4eaefdcefc 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1220,7 +1220,7 @@ static struct purex_item
 	if (IS_QLA29XX(ha))
 		copy_sz = sizeof(struct purex_entry_24xx_ext);
 	else
-		copy_sz = QLA_DEFAULT_PAYLOAD_SIZE;
+		copy_sz = sizeof(struct purex_entry_24xx);
 
 	item = qla24xx_alloc_purex_item(vha, copy_sz);
 	if (!item)
-- 
2.52.0

[PATCH v2 13/13] scsi: qla2xxx: Update version to 12.00.00.2610b1

[Nilesh Javali]

Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_version.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_version.h b/drivers/scsi/qla2xxx/qla_version.h
index ef9dc88221c9..0b0c7a3d23b5 100644
--- a/drivers/scsi/qla2xxx/qla_version.h
+++ b/drivers/scsi/qla2xxx/qla_version.h
@@ -6,9 +6,9 @@
 /*
  * Driver version
  */
-#define QLA2XXX_VERSION      "12.00.00.2607b2"
+#define QLA2XXX_VERSION      "12.00.00.2610b1"
 
 #define QLA_DRIVER_MAJOR_VER	12
 #define QLA_DRIVER_MINOR_VER	00
 #define QLA_DRIVER_PATCH_VER	00
-#define QLA_DRIVER_BETA_VER	2607
+#define QLA_DRIVER_BETA_VER	2610
-- 
2.52.0

Re: [PATCH v2 03/13] Revert "scsi: qla2xxx: Validate BSG request_len before reading vendor_cmd[]"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: qla2xxx: unchecked vendor_cmd[0] read in qla24xx_bsg_request()
- [High] scsi: qla2xxx: out-of-bounds vendor_cmd[] reads on short BSG requests
--

commit 6dca3d90086c395d6e304de559abf4c8c863ad1f
Author: Nilesh Javali <njavali@marvell.com>

Revert "scsi: qla2xxx: Validate BSG request_len before reading vendor_cmd[]"

This commit reverts commit 4cf38dd9465736141263ebb63375868311a0ec81 to
restore FC_BSG_HST_VENDOR handling for the Marvell qaucli management utility,
which passes a request_len smaller than sizeof(struct fc_bsg_request) plus
the vendor_cmd words checked by that commit.

> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index ab559048dbb82..19a514a153aef 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -160,12 +160,6 @@ qla24xx_proc_fcp_prio_cfg_cmd(struct bsg_job *bsg_job)
>  		goto exit_fcp_prio_cfg;
>  	}
>  
> -	if (bsg_job->request_len <
> -	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t)) {
> -		ret = -EINVAL;
> -		goto exit_fcp_prio_cfg;
> -	}
> -
>  	/* Get the sub command */
>  	oper = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
>  
> @@ -764,10 +758,6 @@ qla2x00_process_loopback(struct bsg_job *bsg_job)
>  		return -EIO;
>  	}
>  
> -	if (bsg_job->request_len <
> -	    sizeof(struct fc_bsg_request) + 3 * sizeof(uint32_t))
> -		return -EINVAL;
> -
>  	memset(&elreq, 0, sizeof(elreq));
>  
>  	elreq.req_sg_cnt = dma_map_sg(&ha->pdev->dev,
> @@ -1000,10 +990,6 @@ qla84xx_reset(struct bsg_job *bsg_job)
>  		return -EINVAL;
>  	}
>  
> -	if (bsg_job->request_len <
> -	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
> -		return -EINVAL;
> -
>  	flag = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
>  
>  	rval = qla84xx_reset_chip(vha, flag == A84_ISSUE_RESET_DIAG_FW);
> @@ -1048,10 +1034,6 @@ qla84xx_updatefw(struct bsg_job *bsg_job)
>  		return -EINVAL;
>  	}
>  
> -	if (bsg_job->request_len <
> -	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
> -		return -EINVAL;
> -
>  	sg_cnt = dma_map_sg(&ha->pdev->dev, bsg_job->request_payload.sg_list,
>  		bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);
>  	if (!sg_cnt) {
> @@ -1529,15 +1511,9 @@ qla2x00_read_optrom(struct bsg_job *bsg_job)
>  	struct Scsi_Host *host = fc_bsg_to_shost(bsg_job);
>  	scsi_qla_host_t *vha = shost_priv(host);
>  	struct qla_hw_data *ha = vha->hw;
> -	uint32_t start;
> +	uint32_t start = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
>  	int rval = 0;
>  
> -	if (bsg_job->request_len <
> -	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
> -		return -EINVAL;
> -
> -	start = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
> -
>  	if (ha->flags.nic_core_reset_hdlr_active)
>  		return -EBUSY;
>  
> @@ -1580,15 +1556,9 @@ qla2x00_update_optrom(struct bsg_job *bsg_job)
>  	struct Scsi_Host *host = fc_bsg_to_shost(bsg_job);
>  	scsi_qla_host_t *vha = shost_priv(host);
>  	struct qla_hw_data *ha = vha->hw;
> -	uint32_t start;
> +	uint32_t start = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
>  	int rval = 0;
>  
> -	if (bsg_job->request_len <
> -	    sizeof(struct fc_bsg_request) + 2 * sizeof(uint32_t))
> -		return -EINVAL;
> -
> -	start = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
> -
>  	mutex_lock(&ha->optrom_mutex);
>  	rval = qla2x00_optrom_setup(bsg_job, vha, start, 1);
>  	if (rval) {
> @@ -2453,11 +2423,6 @@ qlafx00_mgmt_cmd(struct bsg_job *bsg_job)
>  	struct fc_port *fcport;
>  	char  *type = "FC_BSG_HST_FX_MGMT";
>  
> -	if (bsg_job->request_len <
> -	    sizeof(struct fc_bsg_request) + sizeof(uint32_t) +
> -	    sizeof(struct qla_mt_iocb_rqst_fx00))
> -		return -EINVAL;
> -
>  	/* Copy the IOCB specific information */
>  	piocb_rqst = (struct qla_mt_iocb_rqst_fx00 *)
>  	    &bsg_request->rqst_data.h_vendor.vendor_cmd[1];
> @@ -3379,13 +3344,6 @@ qla2x00_process_vendor_specific(struct scsi_qla_host *vha, struct bsg_job *bsg_j
>  {
>  	struct fc_bsg_request *bsg_request = bsg_job->request;
>  
> -	if (bsg_job->request_len <
> -	    sizeof(struct fc_bsg_request) + sizeof(uint32_t)) {
> -		ql_log(ql_log_warn, vha, 0x7000,
> -		       "BSG request too small for vendor cmd.\n");
> -		return -EINVAL;
> -	}
> -
>  	ql_dbg(ql_dbg_edif, vha, 0x911b, "%s FC_BSG_HST_VENDOR cmd[0]=0x%x\n",
>  	    __func__, bsg_request->rqst_data.h_vendor.vendor_cmd[0]);

[Severity: High]
Does removing all of the request_len checks reintroduce out-of-bounds heap
reads of bsg_request->rqst_data.h_vendor.vendor_cmd[] when userspace submits
a short FC_BSG_HST_VENDOR request?

In bsg_transport_sg_io_fn(), job->request is allocated with the exact
user-supplied hdr->request_len:

block/bsg-lib.c:bsg_transport_sg_io_fn() {
    ...
    job->request_len = hdr->request_len;
    job->request = memdup_user(uptr64(hdr->request), hdr->request_len);
    ...
}

And fc_bsg_host_dispatch() only verifies that job->request_len is at least
sizeof(uint32_t) (4 bytes) before calling into qla24xx_bsg_request() ->
qla2x00_process_vendor_specific().

In struct fc_bsg_request, rqst_data is a union whose total size is 16 bytes
(sized by struct fc_bsg_host_ct), making sizeof(struct fc_bsg_request) 20
bytes. However, struct fc_bsg_host_vendor only has an 8-byte vendor_id before
the flexible array vendor_cmd[]:

include/uapi/scsi/scsi_bsg_fc.h:
    struct fc_bsg_host_vendor {
        __u64 vendor_id;
        __u32 vendor_cmd[];
    };

So offsetof(struct fc_bsg_request, rqst_data.h_vendor.vendor_cmd) is 12
bytes, meaning vendor_cmd[0] is at bytes 12..15 and vendor_cmd[1] is at bytes
16..19 (both within a 20-byte struct fc_bsg_request). Commit 4cf38dd94657
rejected 20-byte qaucli requests because it checked against
sizeof(struct fc_bsg_request) + N * sizeof(uint32_t) (20 + 4*N) rather than
offsetof(struct fc_bsg_request, rqst_data.h_vendor.vendor_cmd) +
N * sizeof(uint32_t) (12 + 4*N).

Without bounds checks anchored at offsetof(struct fc_bsg_request,
rqst_data.h_vendor.vendor_cmd), short requests read past the end of
bsg_job->request in:
- qla2x00_process_vendor_specific() reading vendor_cmd[0]
- qla24xx_proc_fcp_prio_cfg_cmd(), qla84xx_reset(), qla84xx_updatefw(),
  qla2x00_read_optrom(), qla2x00_update_optrom(), and
  qla28xx_validate_flash_image() -> qla28xx_do_validate_flash_image()
  reading vendor_cmd[1]
- qla2x00_process_loopback() reading vendor_cmd[1] and vendor_cmd[2]
- qlafx00_mgmt_cmd() overlaying a 36-byte struct qla_mt_iocb_rqst_fx00 at
  &vendor_cmd[1]

> @@ -3529,11 +3487,8 @@ qla24xx_bsg_request(struct bsg_job *bsg_job)
>  	}
>  
>  	/* Disable port will bring down the chip, allow enable command */
> -	if (bsg_request->msgcode == FC_BSG_HST_VENDOR &&
> -	    bsg_job->request_len >=
> -		sizeof(struct fc_bsg_request) + sizeof(uint32_t) &&
> -	    (bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_MANAGE_HOST_PORT ||
> -	     bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_GET_HOST_STATS))
> +	if (bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_MANAGE_HOST_PORT ||
> +	    bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_GET_HOST_STATS)
>  		goto skip_chip_chk;

[Severity: High]
In qla24xx_bsg_request(), does dropping the bsg_request->msgcode ==
FC_BSG_HST_VENDOR check and length check cause bsg_request->rqst_data.h_vendor.vendor_cmd[0]
to be read unconditionally on every BSG request, including non-vendor
requests?

In struct fc_bsg_request, rqst_data is a union:

include/uapi/scsi/scsi_bsg_fc.h:
    struct fc_bsg_request {
        __u32 msgcode;
        union {
            struct fc_bsg_host_add_rport    h_addrport;
            struct fc_bsg_host_del_rport    h_delrport;
            struct fc_bsg_host_els          h_els;
            struct fc_bsg_host_ct           h_ct;
            struct fc_bsg_host_vendor       h_vendor;

            struct fc_bsg_rport_els         r_els;
            struct fc_bsg_rport_ct          r_ct;
        } rqst_data;
    } __attribute__((packed));

Because rqst_data.h_vendor.vendor_cmd[0] is at byte offset 12..15:

1. For legitimate non-vendor BSG requests smaller than 16 bytes, such as
   FC_BSG_RPT_ELS (where msgcode + struct fc_bsg_rport_els is 5 bytes) or
   FC_BSG_HST_ELS_NOLOGIN (where msgcode + struct fc_bsg_host_els is 8 bytes),
   reading vendor_cmd[0] reads past the end of the bsg_job->request buffer
   allocated by memdup_user().

2. For FC_BSG_HST_CT and FC_BSG_RPT_CT requests, bytes 12..15 alias
   rqst_data.h_ct.preamble_word1 and rqst_data.r_ct.preamble_word2. If that
   field happens to equal QL_VND_MANAGE_HOST_PORT (0x26) or
   QL_VND_GET_HOST_STATS (0x24), qla24xx_bsg_request() jumps to skip_chip_chk
   and bypasses the port_isolated, qla2x00_chip_is_down(vha), and
   PFLG_DRIVER_REMOVING checks:

drivers/scsi/qla2xxx/qla_bsg.c:qla24xx_bsg_request() {
    ...
    if (bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_MANAGE_HOST_PORT ||
        bsg_request->rqst_data.h_vendor.vendor_cmd[0] == QL_VND_GET_HOST_STATS)
        goto skip_chip_chk;

    if (vha->hw->flags.port_isolated) {
        bsg_reply->result = DID_ERROR;
        /* operation not permitted */
        return -EPERM;
    }

    if (qla2x00_chip_is_down(vha)) {
        ...
        return -EBUSY;
    }

    if (test_bit(PFLG_DRIVER_REMOVING, &vha->pci_flags)) {
        SET_DID_STATUS(bsg_reply->result, DID_ERROR);
        return -EIO;
    }

skip_chip_chk:
    ...
}

Should qla24xx_bsg_request() retain the bsg_request->msgcode ==
FC_BSG_HST_VENDOR check along with a check that bsg_job->request_len >=
offsetof(struct fc_bsg_request, rqst_data.h_vendor.vendor_cmd) +
sizeof(uint32_t)?

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

Re: [PATCH v2 06/13] scsi: qla2xxx: Fix scan state machine race and IRQ-unsafe sp_unmap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: qla2xxx: incomplete scan.step race fix in qla_async_scan_sp_done()
- [High] scsi: qla2xxx: sp and DMA leak in qla_async_scan_sp_done() on work alloc fail
--

commit e9cd982d25a90416902027b8c35c59ac8e31a64e
Author: Manish Rangankar <mrangankar@marvell.com>

scsi: qla2xxx: Fix scan state machine race and IRQ-unsafe sp_unmap

This commit saves vha->scan.step into a local variable under work_lock in
qla_async_scan_sp_done() and defers qla24xx_sp_unmap() to process context via
QLA_EVT_UNMAP when posting scan work fails.

> diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
> index 20b1aef455c4f..efe98609c149d 100644
> --- a/drivers/scsi/qla2xxx/qla_gs.c
> +++ b/drivers/scsi/qla2xxx/qla_gs.c
> @@ -3512,6 +3512,8 @@ static void qla2x00_find_free_fcp_nvme_slot(struct scsi_qla_host *vha,
>  static void qla_async_scan_sp_done(srb_t *sp, int res)
>  {
>  	struct scsi_qla_host *vha = sp->vha;
> +	struct qla_work_evt *e;
> +	enum scan_step step;
>  	unsigned long flags;
>  	int rc;
>  
> @@ -3544,8 +3546,16 @@ static void qla_async_scan_sp_done(srb_t *sp, int res)
>  		 */
>  		rc = qla2x00_post_next_scan_work(vha, sp, QLA_EVT_SCAN_FINISH);
>  		if (rc) {
> -			/* Cleanup here to prevent memory leak */
> -			qla24xx_sp_unmap(vha, sp);
> +			/* Defer sp_unmap to process context -- we are in IRQ */
> +			e = qla2x00_alloc_work(vha, QLA_EVT_UNMAP);
> +			if (e) {
> +				e->u.iosb.sp = sp;
> +				qla2x00_post_work(vha, e);
> +			} else {
> +				ql_log(ql_log_warn, vha, 0x2029,
> +				       "%s: Failed to schedule sp_unmap, sp leaked\n",
> +				       name);
> +			}
>  
>  			spin_lock_irqsave(&vha->work_lock, flags);
>  			vha->scan.scan_flags &= ~SF_SCANNING;
> @@ -3568,10 +3578,11 @@ static void qla_async_scan_sp_done(srb_t *sp, int res)
>  	qla2x00_find_free_fcp_nvme_slot(vha, sp);
>  
>  	spin_lock_irqsave(&vha->work_lock, flags);
> +	step = vha->scan.step;
>  	vha->scan.scan_flags &= ~SF_SCANNING;
>  	spin_unlock_irqrestore(&vha->work_lock, flags);
>  
> -	switch (vha->scan.step) {
> +	switch (step) {

[Severity: High]
Does saving vha->scan.step into a local variable while clearing SF_SCANNING
between scan steps allow two fabric scans to run concurrently and corrupt
vha->scan?

In the race described by the commit message, as soon as SF_SCANNING is
cleared here in qla_async_scan_sp_done(), another CPU calling
qla_fab_scan_start() -> qla_fab_async_scan(vha, NULL) can see SF_SCANNING
clear, set SF_SCANNING, reset vha->scan.step to FAB_SCAN_START, and zero
vha->scan.l:

drivers/scsi/qla2xxx/qla_gs.c:qla_fab_async_scan() {
	spin_lock_irqsave(&vha->work_lock, flags);
	if (vha->scan.scan_flags & SF_SCANNING) {
		spin_unlock_irqrestore(&vha->work_lock, flags);
		ql_dbg(ql_dbg_disc + ql_dbg_verbose, vha, 0x2012,
		    "%s: scan active for sp:%p\n", __func__, sp);
		goto done_free_sp;
	}
	vha->scan.scan_flags |= SF_SCANNING;
	if (!sp)
		vha->scan.step = FAB_SCAN_START;

	spin_unlock_irqrestore(&vha->work_lock, flags);
	...
		memset(vha->scan.l, 0, vha->scan.size);
}

With this change, qla_async_scan_sp_done() switches on the saved step and
posts QLA_EVT_SCAN_CMD or QLA_EVT_SCAN_FINISH for the first scan while the
second scan is also running:

1. If step was FAB_SCAN_GNNFT_NVME (or FAB_SCAN_GNNFT_FCP without NVMe),
   qla_fab_scan_finish() runs on the workqueue concurrently with the new
   scan, reads vha->scan.l while the new scan zeroes or overwrites it (which
   can leave online ports in QLA_FCPORT_SCAN and schedule them for deletion
   via qlt_schedule_sess_for_deletion()), and then clears SF_SCANNING at the
   out label while the second scan's SRB is in flight.

2. If step was an intermediate step, qla_fab_async_scan(vha, sp) runs with
   the first scan's non-NULL sp. If the second scan failed an allocation in
   FAB_SCAN_START (which clears SF_SCANNING while leaving vha->scan.step as
   FAB_SCAN_START), the first scan enters case FAB_SCAN_START and overwrites
   sp = qla2x00_get_sp(vha, NULL, GFP_KERNEL), leaking the first scan's sp
   and its coherent DMA buffers ctarg.req and ctarg.rsp.

3. Conversely, if qla_fab_async_scan() is entered while SF_SCANNING is
   already set, it jumps to done_free_sp, which unconditionally clears
   SF_SCANNING:

drivers/scsi/qla2xxx/qla_gs.c:qla_fab_async_scan() {
done_free_sp:
	...
	spin_lock_irqsave(&vha->work_lock, flags);
	vha->scan.scan_flags &= ~SF_SCANNING;
}

   Once SF_SCANNING is cleared while an SRB is still in flight, another
   qla_fab_scan_start() call can set vha->scan.step = FAB_SCAN_START before
   qla_async_scan_sp_done() runs, so qla2x00_find_free_fcp_nvme_slot() (which
   reads vha->scan.step outside work_lock) and step = vha->scan.step in
   qla_async_scan_sp_done() will still see FAB_SCAN_START and hit WARN_ON(1).

>  	case FAB_SCAN_GPNFT_FCP:
>  	case FAB_SCAN_GPNFT_NVME:
>  		rc = qla2x00_post_next_scan_work(vha, sp, QLA_EVT_SCAN_CMD);
> @@ -3594,7 +3605,16 @@ static void qla_async_scan_sp_done(srb_t *sp, int res)
>  	}
>  
>  	if (rc) {
> -		qla24xx_sp_unmap(vha, sp);
> +		/* Defer sp_unmap to process context -- we are in IRQ */
> +		e = qla2x00_alloc_work(vha, QLA_EVT_UNMAP);
> +		if (e) {
> +			e->u.iosb.sp = sp;
> +			qla2x00_post_work(vha, e);
> +		} else {
> +			ql_log(ql_log_warn, vha, 0x202a,
> +			       "%s: Failed to schedule sp_unmap, sp leaked\n",
> +			       __func__);
> +		}

[Severity: High]
Can qla2x00_alloc_work(vha, QLA_EVT_UNMAP) ever succeed here or in the error
path above when qla2x00_post_next_scan_work() just failed, and does this leak
sp, its DMA buffers, and its vha and qpair references during unload or vport
deletion?

Looking at qla2x00_post_next_scan_work():

drivers/scsi/qla2xxx/qla_gs.c:qla2x00_post_next_scan_work() {
	e = qla2x00_alloc_work(vha, cmd);
	if (!e)
		return QLA_FUNCTION_FAILED;

	e->u.iosb.sp = sp;

	return qla2x00_post_work(vha, e);
}

Since qla2x00_post_work() always returns QLA_SUCCESS,
qla2x00_post_next_scan_work() only fails when qla2x00_alloc_work() returns
NULL:

drivers/scsi/qla2xxx/qla_os.c:qla2x00_alloc_work() {
	if (test_bit(UNLOADING, &vha->dpc_flags))
		return NULL;

	if (qla_vha_mark_busy(vha))
		return NULL;

	e = kzalloc_obj(struct qla_work_evt, GFP_ATOMIC);
	...
}

When an in-flight scan SRB is aborted during driver unload (where UNLOADING
is set before qla2x00_abort_all_cmds() invokes sp->done()) or vport deletion
(where delete_progress causes qla_vha_mark_busy() to fail),
qla2x00_post_next_scan_work() fails because qla2x00_alloc_work() returns
NULL. Calling qla2x00_alloc_work(vha, QLA_EVT_UNMAP) immediately afterwards
checks the exact same conditions and will also return NULL (and under
GFP_ATOMIC allocation failure, retries the identical allocation in the same
hard IRQ context).

When the else branch is taken, sp, sp->u.iocb_cmd.u.ctarg.req, and
sp->u.iocb_cmd.u.ctarg.rsp are leaked, and kref_put(&sp->cmd_kref,
qla2x00_sp_release) is never called, leaving vha->vref_count elevated (which
stalls qla24xx_deallocate_vp_id()) and leaving objects allocated in
srb_cachep when kmem_cache_destroy(srb_cachep) runs in qla2x00_module_exit().

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

Re: [PATCH v2 04/13] scsi: qla2xxx: Wait for session deletion before full-login LIP

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: qla2xxx: qla2x00_loop_reset() stalls 10s and skips FCP-2 session deletion
--

commit 58a127391fc6a3fa18dd4432ba3a5356192f9875
Author: Manish Rangankar <mrangankar@marvell.com>

scsi: qla2xxx: Wait for session deletion before full-login LIP

This commit replaces qla2x00_mark_all_devices_lost() with
qla2x00_wait_for_sess_deletion() in qla2x00_loop_reset() so that fcport
session deletions and workqueue items complete before issuing a full-login
LIP mailbox command.

> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index c0efdbff5da71..0e5e672e81ee6 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -1752,7 +1752,7 @@ qla2x00_loop_reset(scsi_qla_host_t *vha)
>  	if (ha->flags.enable_lip_full_login && !IS_CNA_CAPABLE(ha)) {
>  		atomic_set(&vha->loop_state, LOOP_DOWN);
>  		atomic_set(&vha->loop_down_timer, LOOP_DOWN_TIME);
> -		qla2x00_mark_all_devices_lost(vha);
> +		qla2x00_wait_for_sess_deletion(vha);
>  		ret = qla2x00_full_login_lip(vha);

[Severity: Medium]
When a logged-in FCP-2 target (FCF_FCP2_DEVICE) is present and ql2xfc2target
is enabled (which defaults to 1), will calling
qla2x00_wait_for_sess_deletion() from qla2x00_loop_reset() stall for the full
10-second timeout and still leave the FCP-2 session undeleted when
qla2x00_full_login_lip() is issued?

qla2x00_loop_reset() is called from qla2xxx_eh_bus_reset() (after
qla2x00_wait_for_hba_online() has waited for ISP abort flags to clear) and
from sysfs qla2x00_issue_lip(). In both paths, qla2x00_reset_active(vha) is
false.

When qla2x00_loop_reset() -> qla2x00_wait_for_sess_deletion() calls
qla2x00_mark_all_devices_lost(), any logged-in FCP-2 target is skipped
because !qla2x00_reset_active(vha) is true:

drivers/scsi/qla2xxx/qla_os.c:qla2x00_mark_all_devices_lost() {
	list_for_each_entry(fcport, &vha->vp_fcports, list) {
		if (ql2xfc2target &&
		    fcport->loop_id != FC_NO_LOOP_ID &&
		    (fcport->flags & FCF_FCP2_DEVICE) &&
		    fcport->port_type == FCT_TARGET &&
		    !qla2x00_reset_active(vha)) {
			ql_dbg(ql_dbg_disc, vha, 0x211a,
			       "Delaying session delete for FCP2 flags 0x%x port_type = 0x%x port_id=%06x %phC",
			       fcport->flags, fcport->port_type,
			       fcport->d_id.b24, fcport->port_name);
			continue;
		}
		fcport->scan_state = 0;
		qlt_schedule_sess_for_deletion(fcport);
	}
}

Because qlt_schedule_sess_for_deletion(fcport) is skipped and
qla2x00_full_login_lip(vha) has not been sent yet, vha->fcport_count remains
non-zero and fcport->deleted remains 0. As a result, test_fcport_count(vha)
never becomes true in qla2x00_wait_for_sess_deletion():

drivers/scsi/qla2xxx/qla_os.c:qla2x00_wait_for_sess_deletion() {
	qla2x00_mark_all_devices_lost(vha);

	for (i = 0; i < 10; i++) {
		if (wait_event_timeout(vha->fcport_waitQ,
		    test_fcport_count(vha), HZ) > 0)
			break;
	}

	flush_workqueue(vha->hw->wq);
}

After blocking for all 10 iterations (10 * HZ), qla2x00_full_login_lip(vha)
proceeds with the FCP-2 fcport still in DSC_LOGIN_COMPLETE. If the N2N peer
is an FCP-2 target, RIDA processing in qla24xx_report_id_acquisition() will
then hit the default branch and call qlt_schedule_sess_for_deletion(fcport)
(placing it in DSC_DELETE_PEND) right as N2N_LOGIN_NEEDED is set:

drivers/scsi/qla2xxx/qla_mbx.c:qla24xx_report_id_acquisition() {
				switch (fcport->disc_state) {
				case DSC_DELETED:
					set_bit(RELOGIN_NEEDED,
					    &vha->dpc_flags);
					break;
				case DSC_DELETE_PEND:
					break;
				default:
					qlt_schedule_sess_for_deletion(fcport);
					break;
				}
}

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

Re: [PATCH v2 02/13] scsi: qla2xxx: Update few AENs for 29xx handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] scsi: qla2xxx: missing IS_QLA29XX() in qla24xx and mpi RAM dump helpers
- [Medium] scsi: qla2xxx: unloaded fwdt[1] template breaks 29xx MPI dump in qla2x00_async_event()
--

commit a55d0fe512b854062fc6be4342178f19e1073032
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Update few AENs for 29xx handling

This commit updates qla2x00_async_event() so that QLA29XX adapters follow the
same handling as QLA27XX and QLA28XX adapters for several asynchronous events,
including triggering MPI and RISC firmware dumps on MBA_SYSTEM_ERR and
MBA_IDC_AEN.

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 63b70d4abbf02..7bae26d4404c2 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -1443,7 +1443,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
>  		vha->hw_err_cnt++;
>  
>  		if (IS_QLA81XX(ha) || IS_QLA83XX(ha) ||
> -		    IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
> +		    IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) {
>  			u16 m[4];
>  
>  			m[0] = rd_reg_word(&reg24->mailbox4);
> @@ -1459,7 +1459,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
>  			    "ISP System Error - mbx1=%xh mbx2=%xh mbx3=%xh.\n ",
>  			    mb[1], mb[2], mb[3]);
>  
> -		if ((IS_QLA27XX(ha) || IS_QLA28XX(ha)) &&
> +		if ((IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) &&
>  		    rd_reg_word(&reg24->mailbox7) & BIT_8)
>  			ha->isp_ops->mpi_fw_dump(vha, 1);

[Severity: Medium]
Can ha->isp_ops->mpi_fw_dump(vha, 1) succeed on QLA29XX under the default
flash firmware load path (ql2xfwloadbin == 0)?

When qla29xx_load_risc() loads firmware from flash, it calls
qla29xx_load_fw_template(), which hardcodes templates = 1 and only populates
ha->fwdt[0].template:

drivers/scsi/qla2xxx/qla_init.c:qla29xx_load_fw_template() {
    struct qla_hw_data *ha = vha->hw;
    struct fwdt *fwdt = ha->fwdt;
    uint templates = 1;
    ...
}

Because ha->fwdt[1].template remains NULL, calling qla27xx_mpi_fwdump() via
ha->isp_ops->mpi_fw_dump(vha, 1) during MBA_SYSTEM_ERR (when mailbox7 BIT_8 is
set) or MBA_IDC_AEN will hit the missing template check and bail out:

drivers/scsi/qla2xxx/qla_tmpl.c:qla27xx_mpi_fwdump() {
    ...
    struct fwdt *fwdt = &ha->fwdt[1];
    ...
    if (!fwdt->template) {
        ql_log(ql_log_warn, vha, 0x02f6,
               "-> fwdt1 no template\n");
        goto bailout;
    }
    ...
}

In addition, when template entries execute for QLA29XX,
qla27xx_fwdt_entry_t263() in drivers/scsi/qla2xxx/qla_tmpl.c copies request
and response queues using sizeof(*req->ring) and sizeof(*rsp->ring) (64 bytes)
rather than qla_req_entry_size(vha->hw) and qla_rsp_entry_size(vha->hw) (128
bytes on QLA29XX):

drivers/scsi/qla2xxx/qla_tmpl.c:qla27xx_fwdt_entry_t263() {
    ...
    qla27xx_insertbuf(req ? req->ring : NULL,
        length * sizeof(*req->ring), buf, len);
    ...
    qla27xx_insertbuf(rsp ? rsp->ring : NULL,
        length * sizeof(*rsp->ring), buf, len);
    ...
}

Should qla29xx_load_fw_template() load the secondary MPI template into
ha->fwdt[1] and should qla27xx_fwdt_entry_t263() use the dynamic entry size
helpers so that QLA29XX dumps are not aborted or truncated?

[ ... ]

> @@ -1998,7 +1999,7 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
>  		break;
>  
>  	case MBA_IDC_AEN:
> -		if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
> +		if (IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) {
>  			vha->hw_err_cnt++;
>  			qla27xx_handle_8200_aen(vha, mb);

[Severity: Medium]
When MBA_SYSTEM_ERR or MBA_IDC_AEN (via qla27xx_handle_8200_aen()) triggers
qla27xx_mpi_fwdump() or qla27xx_fwdump() on QLA29XX, does executing template
entries T272 (ENTRY_TYPE_RDREMRAM) and T262 (ENTRY_TYPE_RD_RAM) byte-swap all
dumped RAM words?

qla27xx_execute_fwdt_template() dispatches T272 to qla27xx_dump_mpi_ram() and
T262 to qla24xx_dump_ram(). Both helpers in drivers/scsi/qla2xxx/qla_dbg.c
check only IS_QLA27XX(ha) || IS_QLA28XX(ha) to skip swab32():

drivers/scsi/qla2xxx/qla_dbg.c:qla27xx_dump_mpi_ram() {
    ...
    ram[i + j] =
        (IS_QLA27XX(ha) || IS_QLA28XX(ha)) ?
        chunk[j] : swab32(chunk[j]);
    ...
}

drivers/scsi/qla2xxx/qla_dbg.c:qla24xx_dump_ram() {
    ...
    ram[i + j] = (__force __be32)
        ((IS_QLA27XX(ha) || IS_QLA28XX(ha)) ?
         chunk[j] : swab32(chunk[j]));
    ...
}

Should both of these checks also include IS_QLA29XX(ha) so that MPI and RISC
RAM words in QLA29XX template dumps are not byte-swapped?

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

Re: [PATCH v2 05/13] scsi: qla2xxx: Fix double addition of fcport to gnl.fcport list

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: qla2xxx: FCF_ASYNC_ACTIVE leak on early return in qla24xx_async_gnl()
- [High] scsi: qla2xxx: incomplete gnl_entry double-add fix in qla24xx_async_gnl()
--

commit 92c98a5a2f2d104f2b6d47badef1b553bffd531c
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Fix double addition of fcport to gnl.fcport list

This commit moves the FCF_ASYNC_SENT check in qla24xx_async_gnl() under
vha->hw->tgt.sess_lock to prevent adding fcport->gnl_entry to
vha->gnl.fcports twice.

> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index bb0cc71de37b8..515a7ea4299db 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -1158,13 +1158,17 @@ int qla24xx_async_gnl(struct scsi_qla_host *vha, fc_port_t *fcport)
>  	unsigned long flags;
>  	u16 *mb;
>  
> -	if (!vha->flags.online || (fcport->flags & FCF_ASYNC_SENT))
> +	if (!vha->flags.online)
>  		goto done;
>  
>  	ql_dbg(ql_dbg_disc, vha, 0x20d9,
>  	    "Async-gnlist WWPN %8phC \n", fcport->port_name);
>  
>  	spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
> +	if (fcport->flags & FCF_ASYNC_SENT) {
> +		spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
> +		return QLA_SUCCESS;
> +	}

[Severity: High]
Does returning QLA_SUCCESS directly here leak the FCF_ASYNC_ACTIVE flag on
fcport->flags and cause a permanent session relogin hang?

Every caller that schedules qla24xx_async_gnl() posts a QLA_EVT_GNL work
item via qla24xx_post_gnl_work(), which sets FCF_ASYNC_ACTIVE:

drivers/scsi/qla2xxx/qla_init.c:qla24xx_post_gnl_work() {
	...
	e->u.fcport.fcport = fcport;
	fcport->flags |= FCF_ASYNC_ACTIVE;
	return qla2x00_post_work(vha, e);
}

Previously, when FCF_ASYNC_SENT was set on entry to qla24xx_async_gnl(), it
branched to the done label, which cleared FCF_ASYNC_ACTIVE:

drivers/scsi/qla2xxx/qla_init.c:qla24xx_async_gnl() {
	...
done:
	fcport->flags &= ~(FCF_ASYNC_ACTIVE);
	return rval;
}

With this change, qla24xx_async_gnl() returns QLA_SUCCESS without clearing
FCF_ASYNC_ACTIVE and without adding fcport->gnl_entry to vha->gnl.fcports.

FCF_ASYNC_SENT is a shared per-fcport flag that is also set by other session
management and discovery paths whose completion or cleanup routines only
clear FCF_ASYNC_SENT and never clear FCF_ASYNC_ACTIVE. For example, during
session deletion in drivers/scsi/qla2xxx/qla_target.c, qlt_unreg_sess() sets
FCF_ASYNC_SENT to block discovery commands while a QLA_EVT_GNL work item may
already be queued, and qlt_free_session_done() only clears FCF_ASYNC_SENT:

drivers/scsi/qla2xxx/qla_target.c:qlt_unreg_sess() {
	...
	sess->flags |= FCF_ASYNC_SENT;
	...
}

drivers/scsi/qla2xxx/qla_target.c:qlt_free_session_done() {
	...
	spin_lock_irqsave(&vha->work_lock, flags);
	sess->flags &= ~FCF_ASYNC_SENT;
	sess->deleted = QLA_SESS_DELETED;
	sess->free_pending = 0;
	spin_unlock_irqrestore(&vha->work_lock, flags);
	...
}

Similarly, qla24xx_async_notify_ack() / qla2x00_async_nack_sp_done() in
drivers/scsi/qla2xxx/qla_target.c, qla24xx_async_prli() /
qla2x00_async_prli_sp_done() in drivers/scsi/qla2xxx/qla_init.c, and ISP
reset cleanup in qla2x00_abort_isp_cleanup() and qla_pci_error_cleanup()
only clear FCF_ASYNC_SENT.

When qla24xx_async_gnl() hits this early return while FCF_ASYNC_SENT is set
by any of those paths (or if an in-flight GNL is aborted by chip reset
before qla24xx_async_gnl_sp_done() runs), FCF_ASYNC_ACTIVE remains stuck on
fcport->flags. Both qla24xx_fcport_handle_login() in
drivers/scsi/qla2xxx/qla_init.c and qla2x00_relogin() in
drivers/scsi/qla2xxx/qla_os.c check for FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE
and will defer relogin for that fcport indefinitely, reintroducing the
session hang previously fixed by commit 39d22740712c ("scsi: qla2xxx: Fix
session hang in gnl").

Should this path clear FCF_ASYNC_ACTIVE (or jump to done after unlocking
sess_lock) before returning?

>  	fcport->flags |= FCF_ASYNC_SENT;
>  	qla2x00_set_fcport_disc_state(fcport, DSC_GNL);
>  	fcport->last_rscn_gen = fcport->rscn_gen;

[Severity: High]
Can fcport->gnl_entry still be added twice to vha->gnl.fcports even with the
FCF_ASYNC_SENT check under sess_lock?

First, regarding the race described in the commit message,
qla24xx_async_gnl() is only called from qla2x00_do_work() via
qla2x00_iocb_work_fn() in drivers/scsi/qla2xxx/qla_os.c, which runs on the
single per-vha work item vha->iocb_work (and is further serialized by
IOCB_WORK_ACTIVE in vha->dpc_flags). Because a single work_struct does not
execute concurrently with itself, two QLA_EVT_GNL work items for the same
vha are processed sequentially rather than racing at the entry of
qla24xx_async_gnl().

Instead, FCF_ASYNC_SENT can be cleared to 0 while fcport->gnl_entry is still
linked on vha->gnl.fcports (or on the local list in
qla24xx_async_gnl_sp_done()):

1. In drivers/scsi/qla2xxx/qla_init.c, qla24xx_async_gnl_sp_done() clears
FCF_ASYNC_SENT locklessly before acquiring sess_lock and before unlinking
fcport->gnl_entry (dropping sess_lock around qla24xx_handle_gnl_done_event()
for each entry):

drivers/scsi/qla2xxx/qla_init.c:qla24xx_async_gnl_sp_done() {
	...
	sp->fcport->flags &= ~(FCF_ASYNC_SENT|FCF_ASYNC_ACTIVE);
	...
	spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
	...
	list_splice_init(&vha->gnl.fcports, &h);
	spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);

	while (!list_empty(&h)) {
		spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
		fcport = list_first_entry(&h, struct fc_port, gnl_entry);
		list_del_init(&fcport->gnl_entry);
		spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
		qla24xx_handle_gnl_done_event(vha, fcport, ...);
	}
	...
}

If qla24xx_async_gnl() runs on qla2xxx_wq for sp->fcport during this window,
FCF_ASYNC_SENT is already 0 under sess_lock while sp->fcport->gnl_entry has
not been deleted from the list yet.

2. At the end of qla24xx_async_gnl_sp_done(), when multiple fcports were
queued onto vha->gnl.fcports while vha->gnl.sent was 1, the retrigger loop
only unlinks the first fcport and breaks out, leaving the remaining fcports
linked on vha->gnl.fcports with vha->gnl.sent set to 0:

drivers/scsi/qla2xxx/qla_init.c:qla24xx_async_gnl_sp_done() {
	...
	vha->gnl.sent = 0;
	if (!list_empty(&vha->gnl.fcports)) {
		/* retrigger gnl */
		list_for_each_entry_safe(fcport, tf, &vha->gnl.fcports,
		    gnl_entry) {
			list_del_init(&fcport->gnl_entry);
			fcport->flags &= ~(FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE);
			if (qla24xx_post_gnl_work(vha, fcport) == QLA_SUCCESS)
				break;
		}
	}
	spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);
}

3. Several paths clear FCF_ASYNC_SENT without unlinking fcport->gnl_entry
from vha->gnl.fcports, including ISP reset cleanup in
qla2x00_abort_isp_cleanup() and qla_pci_error_cleanup(), command timeout in
qla2x00_async_iocb_timeout(), session deletion completion in
qlt_free_session_done(), and qla2x00_update_fcport():

drivers/scsi/qla2xxx/qla_init.c:qla2x00_abort_isp_cleanup() {
	...
	list_for_each_entry(fcport, &vha->vp_fcports, list) {
		fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT);
		fcport->scan_state = 0;
	}
	...
}

drivers/scsi/qla2xxx/qla_init.c:qla2x00_async_iocb_timeout() {
	...
	case SRB_MB_IOCB:
		fcport->flags &= ~(FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE);
	...
}

When a subsequent QLA_EVT_GNL work item runs qla24xx_async_gnl() for that
fcport, FCF_ASYNC_SENT is 0 under sess_lock while fcport->gnl_entry is still
linked on vha->gnl.fcports, so list_add_tail() will still trigger the
__list_add_valid_or_report() bug.

Would checking !list_empty(&fcport->gnl_entry) (or using list_move_tail())
under sess_lock before adding fcport->gnl_entry to vha->gnl.fcports avoid
this list corruption?

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

Re: [PATCH v2 12/13] scsi: qla2xxx: Fix purex IOCB copy size on non-29xx adapters

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: qla2xxx: false bug fix claim and bad Fixes tag in qla24xx_copy_std_pkt()
--

commit 80556b286ead6fe5fe43e6fbf646d23656fb7c8a
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Fix purex IOCB copy size on non-29xx adapters

This patch replaces QLA_DEFAULT_PAYLOAD_SIZE with
sizeof(struct purex_entry_24xx) in qla24xx_copy_std_pkt() for non-29xx
adapters and removes the QLA_DEFAULT_PAYLOAD_SIZE macro definition.

>     scsi: qla2xxx: Fix purex IOCB copy size on non-29xx adapters
>     
>     qla24xx_copy_std_pkt() copies a single purex IOCB from the request/
>     response ring into a purex_item. The source entry size depends on the
>     adapter: 64 bytes (struct purex_entry_24xx) on 27xx/28xx and 128 bytes
>     (struct purex_entry_24xx_ext) on 29xx.
>     
>     For non-29xx adapters the copy size was set to QLA_DEFAULT_PAYLOAD_SIZE
>     (128), causing memcpy() to read 64 bytes past the end of the 64-byte
>     source IOCB in the ring. Copy sizeof(struct purex_entry_24xx) instead so
>     exactly one IOCB is copied on these adapters.
>     
>     Cc: stable@vger.kernel.org
>     Fixes: 150a29ebda52 ("scsi: qla2xxx: Enhance purex_entry handling for 29xx series")
>     Signed-off-by: Nilesh Javali <njavali@marvell.com>
> 
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index 97e2a1a9ce3f3..da5ca604d99eb 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -5005,7 +5005,6 @@ struct active_regions {
>  #define QLA_SET_DATA_RATE_NOLR	1
>  #define QLA_SET_DATA_RATE_LR	2 /* Set speed and initiate LR */
>  
> -#define QLA_DEFAULT_PAYLOAD_SIZE	64
>  #define QLA_MAX_IOCB_SIZE		128
>  /*
>   * This item might be allocated with a size > sizeof(struct purex_item).
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 7bae26d4404c2..2a4eaefdcefc2 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -1220,7 +1220,7 @@ static struct purex_item
>  	if (IS_QLA29XX(ha))
>  		copy_sz = sizeof(struct purex_entry_24xx_ext);
>  	else
> -		copy_sz = QLA_DEFAULT_PAYLOAD_SIZE;
> +		copy_sz = sizeof(struct purex_entry_24xx);

[Severity: Medium]
Is the commit description accurate about QLA_DEFAULT_PAYLOAD_SIZE being 128
and causing an out-of-bounds read in qla24xx_copy_std_pkt()?

As shown in the diff for drivers/scsi/qla2xxx/qla_def.h above,
QLA_DEFAULT_PAYLOAD_SIZE is defined as 64 (whereas QLA_MAX_IOCB_SIZE is 128),
and struct purex_entry_24xx in drivers/scsi/qla2xxx/qla_fw.h is also 64 bytes
(enforced by BUILD_BUG_ON(sizeof(struct purex_entry_24xx) != 64) in
drivers/scsi/qla2xxx/qla_os.c).

Commit 7db93e3c580d ("scsi: qla2xxx: Enhance purex_entry handling for 29xx
series") already updated qla24xx_copy_std_pkt() in
drivers/scsi/qla2xxx/qla_isr.c from copying sizeof(item->iocb) to copying
copy_sz (64 bytes via QLA_DEFAULT_PAYLOAD_SIZE on non-29xx adapters).
Replacing QLA_DEFAULT_PAYLOAD_SIZE with sizeof(struct purex_entry_24xx)
therefore does not change the copy size or runtime behavior.

Additionally, the commit hash 150a29ebda52 in the Fixes tag does not exist in
the upstream tree (the upstream commit with that subject is 7db93e3c580d).
Since this change is a cleanup with no functional impact, should the Fixes
and Cc: stable@vger.kernel.org tags be dropped and the commit message updated
to reflect that this is a cleanup?

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

Re: [PATCH v2 11/13] scsi: qla2xxx: Re-acquire vport_slock and refresh tvp during vport DPC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] scsi: qla2xxx: use-after-free on vp in qla2x00_do_dpc_all_vps() on timeout
- [Critical] scsi: qla2xxx: unrefreshed tvp in vp_list loops causes hard lockup and UAF
--

commit a9b1443e24db2e5c7bc6bc02893d52ce93b6f5cf
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Re-acquire vport_slock and refresh tvp during vport DPC

This commit re-acquires vport_slock using a spin_trylock_irqsave() and
cond_resched() loop in qla2x00_do_dpc_all_vps(), refreshes the next list entry
tvp after re-locking in qla2x00_do_dpc_all_vps() and qla2x00_alert_all_vps(),
and adds a cond_resched() before qla2x00_loop_resync() in qla2x00_do_dpc_vp().

> diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
> index 754c806dac807..65dcfc1b04ef9 100644
> --- a/drivers/scsi/qla2xxx/qla_mid.c
> +++ b/drivers/scsi/qla2xxx/qla_mid.c
> @@ -315,6 +315,10 @@ qla2x00_alert_all_vps(struct rsp_que *rsp, uint16_t *mb)
>  			/* Reference held under vport_slock keeps vha valid. */
>  			atomic_dec(&vha->vref_count);
>  			wake_up(&vha->vref_waitq);
> +			/* refresh tvp from the current list state */
> +			if (list_empty(&vha->list))
> +				break;
> +			tvp = list_next_entry(vha, list);
>  		}
>  		i++;
>  	}

[ ... ]

> @@ -446,11 +451,28 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
>  			spin_unlock_irqrestore(&ha->vport_slock, flags);
>  
>  			qla2x00_do_dpc_vp(vp);
> -
> -			spin_lock_irqsave(&ha->vport_slock, flags);
> -			/* Check if vp is still valid before accessing it */
> -			if (vp->vp_idx && !vp->flags.delete_progress)
> -				atomic_dec(&vp->vref_count);
> +			cond_resched();
> +
> +			/* Re-acquire lock with yield to avoid hard lockup if holder is delayed */
> +			while (!spin_trylock_irqsave(&ha->vport_slock, flags))
> +				cond_resched();
> +			/* Reference held under vport_slock keeps vp valid. */
> +			atomic_dec(&vp->vref_count);
> +			wake_up(&vp->vref_waitq);
> +			/*
> +			 * While the lock was dropped, the next entry (tvp) may
> +			 * have been removed via list_del_init(), making its
> +			 * list.next point to itself. list_for_each_entry_safe
> +			 * would then loop infinitely since the self-referencing
> +			 * pointer never matches the list head.
> +			 *
> +			 * If vp itself was removed (deallocate timeout), we
> +			 * cannot continue iterating from it, break out.
> +			 * Otherwise refresh tvp from the current list state.
> +			 */
> +			if (list_empty(&vp->list))
> +				break;

[Severity: Critical]
Can accessing vp here (and vha in qla2x00_alert_all_vps() above) trigger a
use-after-free when qla24xx_deallocate_vp_id() times out?

Neither qla2x00_do_dpc_all_vps() nor qla2x00_alert_all_vps() takes a
Scsi_Host reference via scsi_host_get() before dropping ha->vport_slock.
If a concurrent vport deletion in qla24xx_vport_delete() calls
qla24xx_deallocate_vp_id() and the wait for vref_count to reach zero times
out, qla24xx_deallocate_vp_id() forcibly removes vha from ha->vp_list even
while vref_count is still non-zero:

drivers/scsi/qla2xxx/qla_mid.c:qla24xx_deallocate_vp_id() {
	...
	if (!bailout) {
		ql_log(ql_log_info, vha, 0xfffa,
			"vha->vref_count=%u timeout\n", vha->vref_count.counter);
		spin_lock_irqsave(&ha->vport_slock, flags);
		list_del_init(&vha->list);
		qla_update_vp_map(vha, RESET_VP_IDX);
		spin_unlock_irqrestore(&ha->vport_slock, flags);
	}
}

Immediately after qla24xx_deallocate_vp_id() returns, qla24xx_vport_delete()
frees vha's resources and drops the final host reference:

drivers/scsi/qla2xxx/qla_attr.c:qla24xx_vport_delete() {
	...
	qla24xx_deallocate_vp_id(vha);
	...
	scsi_host_put(vha->host);
}

Because list_empty(&vp->list) only becomes true while vref_count > 0 when that
deallocate timeout occurs, wouldn't vp (or vha in qla2x00_alert_all_vps())
already be freed or in the middle of being freed by the time this thread
re-acquires ha->vport_slock and accesses vp->vref_count, vp->vref_waitq, and
vp->list?

> +			tvp = list_next_entry(vp, list);

[Severity: Critical]
What about the other list_for_each_entry_safe(vp, tvp, &ha->vp_list, list)
loops that drop and re-acquire ha->vport_slock without refreshing tvp?

Since qla24xx_deallocate_vp_id() uses list_del_init(&vha->list) when
vref_count == 0, any concurrent deletion of the prefetched next entry tvp
while ha->vport_slock is dropped sets tvp->list.next to point to itself and
frees tvp via qla24xx_vport_delete() -> scsi_host_put().

The following functions still iterate over ha->vp_list with
list_for_each_entry_safe(), drop ha->vport_slock inside the loop body, and do
not refresh tvp after re-acquiring the lock:

drivers/scsi/qla2xxx/qla_init.c:qla2x00_quiesce_io()
drivers/scsi/qla2xxx/qla_init.c:qla2x00_abort_isp_cleanup() (two loops)
drivers/scsi/qla2xxx/qla_init.c:qla2x00_abort_isp()
drivers/scsi/qla2xxx/qla_init.c:qla82xx_restart_isp()
drivers/scsi/qla2xxx/qla_os.c:qla_pci_error_cleanup() (two loops)

For example, in qla2x00_abort_isp():

drivers/scsi/qla2xxx/qla_init.c:qla2x00_abort_isp() {
	...
	spin_lock_irqsave(&ha->vport_slock, flags);
	list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
		if (vp->vp_idx && !vp->flags.delete_progress) {
			atomic_inc(&vp->vref_count);
			spin_unlock_irqrestore(&ha->vport_slock, flags);

			qla2x00_vp_abort_isp(vp);

			spin_lock_irqsave(&ha->vport_slock, flags);
			/* Reference held under vport_slock keeps vp valid. */
			atomic_dec(&vp->vref_count);
			wake_up(&vp->vref_waitq);
		}
	}
	spin_unlock_irqrestore(&ha->vport_slock, flags);
}

If tvp is deleted while ha->vport_slock is dropped in any of those loops,
won't the next iteration dereference the freed tvp and then spin forever with
interrupts disabled because tvp->list.next points to itself?

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

Re: [PATCH v2 10/13] scsi: qla2xxx: Fix use-after-free races during vport deletion

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] scsi: qla2xxx: infinite loop and UAF in vp_list loops with list_del_init()
- [High] scsi: qla2xxx: missing vport validation and tvp refresh in vp_list loops
--

commit 24f27bc8c1d9341721d99c592298135dfbc0b9d5
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Fix use-after-free races during vport deletion

This commit adds vport validation checks (vp_idx, delete_progress, and
VPORT_DELETE) to several ha->vp_list traversals and switches
qla24xx_deallocate_vp_id() from list_del() to list_del_init() to prevent
use-after-free crashes during vport deletion.

> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index 7898521b76d4d..c02571a6f0c71 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -7573,15 +7573,17 @@ qla2x00_abort_isp_cleanup(scsi_qla_host_t *vha)
>  
>  		spin_lock_irqsave(&ha->vport_slock, flags);
>  		list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
> -			atomic_inc(&vp->vref_count);
> -			spin_unlock_irqrestore(&ha->vport_slock, flags);
> +			if (vp->vp_idx && !vp->flags.delete_progress) {
> +				atomic_inc(&vp->vref_count);
> +				spin_unlock_irqrestore(&ha->vport_slock, flags);
>  
> -			qla2x00_mark_all_devices_lost(vp);
> +				qla2x00_mark_all_devices_lost(vp);
>  
> -			spin_lock_irqsave(&ha->vport_slock, flags);
> -			/* Check if vp is still valid before accessing it */
> -			if (vp->vp_idx && !vp->flags.delete_progress)
> +				spin_lock_irqsave(&ha->vport_slock, flags);
> +				/* Reference held under vport_slock keeps vp valid. */
>  				atomic_dec(&vp->vref_count);
> +				wake_up(&vp->vref_waitq);
> +			}
>  		}
>  		spin_unlock_irqrestore(&ha->vport_slock, flags);

[Severity: High]
The commit message notes that it validates vp_idx and delete_progress across
all remaining vp_list traversals, but what about qla2x00_quiesce_io() and
qla82xx_restart_isp() in drivers/scsi/qla2xxx/qla_init.c?

In qla2x00_quiesce_io(), ha->vp_list is traversed and ha->vport_slock is
dropped without checking vp->vp_idx or !vp->flags.delete_progress, and
without waking vp->vref_waitq:

drivers/scsi/qla2xxx/qla_init.c:qla2x00_quiesce_io() {
    ...
		spin_lock_irqsave(&ha->vport_slock, flags);
		list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
			atomic_inc(&vp->vref_count);
			spin_unlock_irqrestore(&ha->vport_slock, flags);

			qla2x00_mark_all_devices_lost(vp);

			spin_lock_irqsave(&ha->vport_slock, flags);
			atomic_dec(&vp->vref_count);
		}
		spin_unlock_irqrestore(&ha->vport_slock, flags);
    ...
}

Similarly, qla82xx_restart_isp() checks vp->vp_idx (which is not cleared by
qla24xx_deallocate_vp_id()), but does not check !vp->flags.delete_progress or
call wake_up(&vp->vref_waitq):

drivers/scsi/qla2xxx/qla_init.c:qla82xx_restart_isp() {
    ...
		spin_lock_irqsave(&ha->vport_slock, flags);
		list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) {
			if (vp->vp_idx) {
				atomic_inc(&vp->vref_count);
				spin_unlock_irqrestore(&ha->vport_slock, flags);

				qla2x00_vp_abort_isp(vp);

				spin_lock_irqsave(&ha->vport_slock, flags);
				atomic_dec(&vp->vref_count);
			}
		}
		spin_unlock_irqrestore(&ha->vport_slock, flags);
    ...
}

Since NPIV is supported on QLA8031, QLA82XX, and QLA8044 adapters, if
qla24xx_vport_delete() runs concurrently with qla2x00_quiesce_io() or
qla82xx_restart_isp() and deletes the prefetched next entry tvp while
ha->vport_slock is dropped, won't the next iteration advance vp to the
removed and freed tvp, enter the loop body because !vp->flags.delete_progress
is not checked, and access freed vp and vp->vp_fcports memory in
qla2x00_mark_all_devices_lost(vp) or qla2x00_vp_abort_isp(vp)?

> diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
> index 90f09d36705a6..754c806dac807 100644
> --- a/drivers/scsi/qla2xxx/qla_mid.c
> +++ b/drivers/scsi/qla2xxx/qla_mid.c
> @@ -79,7 +79,7 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha)
>  	for (i = 0; i < 500; i++) {
>  		spin_lock_irqsave(&ha->vport_slock, flags);
>  		if (atomic_read(&vha->vref_count) == 0) {
> -			list_del(&vha->list);
> +			list_del_init(&vha->list);
>  			qla_update_vp_map(vha, RESET_VP_IDX);
>  			bailout = 1;
>  		}
> @@ -94,7 +94,7 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha)
>  		ql_log(ql_log_info, vha, 0xfffa,
>  			"vha->vref_count=%u timeout\n", vha->vref_count.counter);
>  		spin_lock_irqsave(&ha->vport_slock, flags);
> -		list_del(&vha->list);
> +		list_del_init(&vha->list);
>  		qla_update_vp_map(vha, RESET_VP_IDX);
>  		spin_unlock_irqrestore(&ha->vport_slock, flags);
>  	}

[Severity: Critical]
Can changing list_del(&vha->list) to list_del_init(&vha->list) in
qla24xx_deallocate_vp_id() still trigger a use-after-free and also cause an
infinite loop with interrupts disabled in the list_for_each_entry_safe()
traversals?

When ha->vp_list is traversed using list_for_each_entry_safe(vp, tvp,
&ha->vp_list, list) in qla2x00_abort_isp_cleanup(), qla2x00_abort_isp(),
qla_pci_error_cleanup(), qla2x00_do_dpc_all_vps(), qla2x00_alert_all_vps(),
qla2x00_quiesce_io(), and qla82xx_restart_isp(), the macro prefetches the
next entry tvp = list_next_entry(vp, list) before running the loop body. The
loop body only increments vp->vref_count (leaving tvp->vref_count at 0) and
then drops ha->vport_slock.

While ha->vport_slock is dropped during vp processing, a concurrent
qla24xx_vport_delete() -> qla24xx_deallocate_vp_id(tvp) can acquire
ha->vport_slock, observe atomic_read(&tvp->vref_count) == 0, unlink tvp with
list_del_init(&tvp->list), and release ha->vport_slock, after which
qla24xx_vport_delete() frees tvp via scsi_host_put(tvp->host).

When the traversal thread finishes processing vp, re-acquires ha->vport_slock
with spin_lock_irqsave(), and advances the loop (vp = tvp, tvp =
list_next_entry(tvp, list)):

1. It dereferences tvp after it has been unlinked and potentially freed by
scsi_host_put(tvp->host), causing a use-after-free on tvp->list.next and
subsequently on vp->vp_idx, vp->flags.delete_progress, or vp->dpc_flags.

2. Because list_del_init(&tvp->list) sets tvp->list.next = &tvp->list,
list_next_entry(tvp, list) evaluates to tvp itself. Since &tvp->list !=
&ha->vp_list, the loop termination condition is never met, and subsequent
iterations skip the body (because delete_progress or VPORT_DELETE is set)
without dropping ha->vport_slock, spinning forever on the self-linked entry
with local interrupts disabled.

A follow-up commit in the series, commit a9b1443e24db ("scsi: qla2xxx:
Re-acquire vport_slock and refresh tvp during vport DPC"), refreshes tvp =
list_next_entry(vp, list) after re-acquiring ha->vport_slock in
qla2x00_do_dpc_all_vps() and qla2x00_alert_all_vps(), but leaves the
list_for_each_entry_safe() loops in qla2x00_abort_isp_cleanup(),
qla2x00_abort_isp(), qla_pci_error_cleanup(), qla2x00_quiesce_io(), and
qla82xx_restart_isp() without tvp refreshes. Should tvp be refreshed after
re-acquiring ha->vport_slock in all of those loops as well?

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