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

[PATCH 00/14] 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.

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 (10):
  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: Yield work_lock in fabric scan path
  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      | 32 ++++++++++++++---
 drivers/scsi/qla2xxx/qla_init.c    | 40 ++++++++++++++-------
 drivers/scsi/qla2xxx/qla_isr.c     | 16 +++++----
 drivers/scsi/qla2xxx/qla_mid.c     | 33 ++++++++++++++---
 drivers/scsi/qla2xxx/qla_os.c      | 32 ++++++++++-------
 drivers/scsi/qla2xxx/qla_version.h |  4 +--
 10 files changed, 125 insertions(+), 98 deletions(-)


base-commit: 12e67eb89eb2b9516685c744d3f7de0a2d1bd701
-- 
2.52.0

[PATCH 01/14] 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 02/14] 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 03/14] 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: qla2xxx: unchecked request_len in qla2x00_process_vendor_specific()
--

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

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

This commit reverts commit 4cf38dd9465736141263ebb63375868311a0ec81, which
added bsg_job->request_len validation before reading elements of
vendor_cmd[] in drivers/scsi/qla2xxx/qla_bsg.c, because those checks
rejected legitimate requests from the qaucli management utility.

> 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]);
>  

[ ... ]

> @@ -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)

[Severity: High]
Does removing all of these bsg_job->request_len checks without a replacement
reintroduce out-of-bounds heap reads of
bsg_request->rqst_data.h_vendor.vendor_cmd[]?

In block/bsg-lib.c, bsg_transport_sg_io_fn() allocates job->request with the
exact user-supplied hdr->request_len:

    job->request = memdup_user(uptr64(hdr->request), hdr->request_len);

In drivers/scsi/scsi_transport_fc.c, fc_bsg_host_dispatch() only checks
job->request_len < sizeof(uint32_t) before matching FC_BSG_HST_VENDOR and
checking vendor_id, without verifying that job->request_len covers any
elements of the flexible array rqst_data.h_vendor.vendor_cmd[].

Without length checks in drivers/scsi/qla2xxx/qla_bsg.c, a short
FC_BSG_HST_VENDOR request (for example 12 or 16 bytes) can read past the end
of bsg_job->request in:

- qla2x00_process_vendor_specific(), which reads vendor_cmd[0] (bytes 12..15)
- 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(), which
  read vendor_cmd[1] (bytes 16..19)
- qla2x00_process_loopback(), which reads vendor_cmd[1] and vendor_cmd[2]
  (bytes 16..23) later in the function:

	elreq.options = bsg_request->rqst_data.h_vendor.vendor_cmd[1];
	elreq.iteration_count =
	    bsg_request->rqst_data.h_vendor.vendor_cmd[2];

- qlafx00_mgmt_cmd(), which overlays and reads the 32-byte
  struct qla_mt_iocb_rqst_fx00 starting at vendor_cmd[1] (bytes 16..47)

Commit 4cf38dd9465736141263ebb63375868311a0ec81 over-estimated the header
size by 8 bytes because sizeof(struct fc_bsg_request) is 20 bytes (sized by
the 16-byte h_ct union member), whereas
offsetof(struct fc_bsg_request, rqst_data.h_vendor.vendor_cmd) is 12 bytes.

Could these checks be updated to use
offsetof(struct fc_bsg_request, rqst_data.h_vendor.vendor_cmd) as the base
offset instead of reverting them completely, so that qaucli requests succeed
while still preventing out-of-bounds reads of vendor_cmd[]?

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

[PATCH 04/14] 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 05/14] 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.

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

Re: [PATCH 05/14] 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 fix for gnl_entry double add in qla24xx_async_gnl()
--

commit 26e3803dac33bf8507fb3ff3170def3e0261a1f3
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 so that testing and setting FCF_ASYNC_SENT is
serialized with adding fcport->gnl_entry to vha->gnl.fcports.

> 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]
Could returning QLA_SUCCESS directly here instead of jumping to done leak
FCF_ASYNC_ACTIVE on fcport->flags and cause a session relogin hang?

When qla24xx_post_gnl_work() queues a QLA_EVT_GNL work item, it sets
FCF_ASYNC_ACTIVE on fcport:

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, commit 39d22740712c ("scsi: qla2xxx: Fix session hang in gnl")
made the FCF_ASYNC_SENT check in qla24xx_async_gnl() jump to done so that
FCF_ASYNC_ACTIVE would be cleared when bailing out:

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

Because FCF_ASYNC_SENT is also set by non-GNL paths that do not clear
FCF_ASYNC_ACTIVE upon completion, such as qlt_unreg_sess() ->
qlt_free_session_done() in drivers/scsi/qla2xxx/qla_target.c:

	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);

as well as qla24xx_async_prli() -> qla2x00_async_prli_sp_done(),
qla24xx_async_notify_ack() -> qla2x00_async_nack_sp_done(), and
qla2x00_abort_isp_cleanup(), returning early without clearing
FCF_ASYNC_ACTIVE or adding fcport to vha->gnl.fcports leaves
FCF_ASYNC_ACTIVE permanently set once the other operation finishes.

When FCF_ASYNC_ACTIVE remains set, both qla24xx_fcport_handle_login() and
qla2x00_relogin() defer login indefinitely:

drivers/scsi/qla2xxx/qla_init.c:qla24xx_fcport_handle_login() {
	...
	if (fcport->flags & (FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE)) {
		set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
		return 0;
	}
	...
}

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

[Severity: High]
Is checking FCF_ASYNC_SENT under sess_lock sufficient to prevent adding an
already-linked fcport->gnl_entry to vha->gnl.fcports?

All QLA_EVT_GNL work items for a vha are dispatched from the single per-vha
work item vha->iocb_work via qla2x00_iocb_work_fn() -> qla2x00_do_work(), so
two calls to qla24xx_async_gnl() on the same vha do not run concurrently.

Instead, fcport->flags can have FCF_ASYNC_SENT cleared while
fcport->gnl_entry is still linked on vha->gnl.fcports:

1. In drivers/scsi/qla2xxx/qla_init.c, qla24xx_async_gnl_sp_done() clears
FCF_ASYNC_SENT on sp->fcport without holding sess_lock and before splicing
and unlinking entries from vha->gnl.fcports:

	sp->fcport->flags &= ~(FCF_ASYNC_SENT|FCF_ASYNC_ACTIVE);
	...
	spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);

	INIT_LIST_HEAD(&h);
	fcport = tf = NULL;
	if (!list_empty(&vha->gnl.fcports))
		list_splice_init(&vha->gnl.fcports, &h);
	spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);

2. During ISP abort cleanup, qla2x00_abort_isp_cleanup() in
drivers/scsi/qla2xxx/qla_init.c clears FCF_ASYNC_SENT across vp_fcports
without holding sess_lock or unlinking fcport->gnl_entry:

	list_for_each_entry(fcport, &vha->vp_fcports, list) {
		fcport->flags &= ~(FCF_LOGIN_NEEDED | FCF_ASYNC_SENT);
		fcport->scan_state = 0;
	}

The same happens in qla2x00_async_iocb_timeout() (before the asynchronous
abort completes), qlt_free_session_done(), and qla2x00_update_fcport().

3. If qla2x00_get_sp() fails in qla24xx_async_gnl() after
list_add_tail(&fcport->gnl_entry, &vha->gnl.fcports), jumping to done leaves
fcport->gnl_entry linked on vha->gnl.fcports with vha->gnl.sent == 1, and in
qla24xx_async_gnl_sp_done(), the retrigger loop only unlinks the first entry
before breaking with vha->gnl.sent = 0:

	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;
		}
	}

In any of those cases, when a subsequent QLA_EVT_GNL runs
qla24xx_async_gnl(), FCF_ASYNC_SENT is clear even under sess_lock, so
list_add_tail(&fcport->gnl_entry, &vha->gnl.fcports) will still be called on
an already-linked gnl_entry. Should qla24xx_async_gnl() also guard against
!list_empty(&fcport->gnl_entry) under sess_lock and ensure gnl_entry is
unlinked on failure and reset paths?

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

[PATCH 06/14] 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

Re: [PATCH 06/14] 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: guaranteed sp leak in qla_async_scan_sp_done() error fallback
- [High] scsi: qla2xxx: concurrent fabric scan race in qla_async_scan_sp_done()
--

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

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

This commit updates qla_async_scan_sp_done() to snapshot vha->scan.step under
vha->work_lock before switching on it, and replaces direct qla24xx_sp_unmap()
calls in IRQ context with QLA_EVT_UNMAP work events 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);
> +			}

[Severity: High]
Can qla2x00_alloc_work(vha, QLA_EVT_UNMAP) ever succeed when
qla2x00_post_next_scan_work() has just failed, both here in the if (res)
branch and in the if (rc) fallback at the bottom of qla_async_scan_sp_done()?

Looking at qla2x00_post_next_scan_work() in drivers/scsi/qla2xxx/qla_gs.c:

    static int qla2x00_post_next_scan_work(struct scsi_qla_host *vha,
        srb_t *sp, int cmd)
    {
            struct qla_work_evt *e;

            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() unconditionally returns QLA_SUCCESS,
qla2x00_post_next_scan_work() only fails when qla2x00_alloc_work() returns
NULL.

In drivers/scsi/qla2xxx/qla_os.c, qla2x00_alloc_work() checks conditions that
do not depend on the work event type:

    struct qla_work_evt *
    qla2x00_alloc_work(struct scsi_qla_host *vha, enum qla_work_type type)
    {
            struct qla_work_evt *e;

            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);
            if (!e) {
                    QLA_VHA_MARK_NOT_BUSY(vha);
                    return NULL;
            }
            ...
    }

During PCI removal (qla2x00_remove_one()), shutdown (qla2x00_shutdown()), or
vport deletion (qla24xx_vport_delete() in drivers/scsi/qla2xxx/qla_attr.c),
UNLOADING or vha->flags.delete_progress = 1 is set before in-flight commands
are aborted via:

    qla2x00_abort_isp_cleanup() / qla2x00_abort_all_cmds()
      -> qla2x00_abort_srb()
        -> sp->done()
          -> qla_async_scan_sp_done()

When qla2x00_post_next_scan_work() fails in these teardown paths, calling
qla2x00_alloc_work(vha, QLA_EVT_UNMAP) immediately afterward hits the exact
same UNLOADING or delete_progress check and deterministically returns NULL
(and under GFP_ATOMIC memory pressure in IRQ context, retries the identical
allocation that just failed).

Won't this leak sp (from srb_mempool / srb_cachep), its coherent DMA buffers
(sp->u.iocb_cmd.u.ctarg.req and sp->u.iocb_cmd.u.ctarg.rsp), qpair->ref_count,
and vha->vref_count, causing qla24xx_deallocate_vp_id() to stall for its full
10-second timeout on vha->vref_count during vport deletion and triggering a
slab cache teardown warning in kmem_cache_destroy(srb_cachep) on module
unload?

>  
>  			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);

[Severity: High]
If another CPU starts a new scan as soon as SF_SCANNING is cleared here,
does switching on the saved step allow two fabric scans to run concurrently
and corrupt vha->scan?

A fabric scan is a multi-step sequence (FAB_SCAN_START -> FAB_SCAN_GPNFT_FCP
-> FAB_SCAN_GNNFT_FCP -> FAB_SCAN_GPNFT_NVME -> FAB_SCAN_GNNFT_NVME ->
qla_fab_scan_finish()) that reuses a single sp and populates the shared array
vha->scan.l across steps.

When qla_async_scan_sp_done() clears SF_SCANNING and releases vha->work_lock,
a concurrent qla_fab_scan_start() -> qla_fab_async_scan(vha, NULL) on another
CPU can observe 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);
                    ...
                    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);
    }

Because qla_async_scan_sp_done() now continues using the saved step instead
of aborting, the first scan queues QLA_EVT_SCAN_FINISH or QLA_EVT_SCAN_CMD
while the second scan is active:

1. If the first scan queues QLA_EVT_SCAN_FINISH, qla_fab_scan_finish() runs
   without checking SF_SCANNING while the new scan zeroes and repopulates
   vha->scan.l. Can this cause qla_fab_scan_finish() to treat existing
   FCS_ONLINE ports as missing from vha->scan.l and schedule active sessions
   for deletion via qlt_schedule_sess_for_deletion(), before clearing
   SF_SCANNING at out: while the second scan's sp is still in flight?

2. If the first scan queues QLA_EVT_SCAN_CMD (calling qla_fab_async_scan(vha,
   sp) with sp != NULL), if the second scan holds SF_SCANNING, the first scan
   jumps to done_free_sp:, which frees the first sp and unconditionally
   clears SF_SCANNING while the second scan is in flight:

    done_free_sp:
            if (sp)
                    qla24xx_sp_unmap(vha, sp);
            spin_lock_irqsave(&vha->work_lock, flags);
            vha->scan.scan_flags &= ~SF_SCANNING;

   Alternatively, if the second scan failed allocation in FAB_SCAN_START (so
   SF_SCANNING is clear and vha->scan.step is FAB_SCAN_START), the first scan
   enters case FAB_SCAN_START: with sp != NULL and overwrites sp =
   qla2x00_get_sp(), leaking the passed-in sp and its DMA buffers.

3. In addition, qla2x00_find_free_fcp_nvme_slot(vha, sp) is called just
   before spin_lock_irqsave(&vha->work_lock, flags) and reads vha->scan.step
   without holding vha->work_lock. Since any concurrent
   qla_fab_async_scan(vha, NULL) that sees SF_SCANNING set jumps to
   done_free_sp: and clears SF_SCANNING, another scan can still start and
   overwrite vha->scan.step to FAB_SCAN_START before
   qla2x00_find_free_fcp_nvme_slot() or qla_async_scan_sp_done() reads it.

Should SF_SCANNING remain set across the entire multi-step scan until
qla_fab_scan_finish() completes (or the scan aborts), without clearing
SF_SCANNING in done_free_sp: when bailing out on an already-active scan?

[ ... ]

> @@ -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;

This is the second qla2x00_alloc_work(vha, QLA_EVT_UNMAP) fallback mentioned
above that will fail for the same reason as qla2x00_post_next_scan_work() and
leak sp.

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

[PATCH 07/14] 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

Re: [PATCH 07/14] scsi: qla2xxx: Skip vport DPC processing during vport deletion

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: qla2xxx: wrong vha delete_progress check in qla2x00_do_dpc_all_vps()
--

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

scsi: qla2xxx: Skip vport DPC processing during vport deletion

This commit attempts to prevent a race between virtual port dpc processing in
qla2x00_do_dpc_all_vps() and virtual port deletion by checking the
delete_progress flag before invoking qla2x00_do_dpc_vp(). A later commit in
the series ("scsi: qla2xxx: Fix use-after-free races during vport deletion")
updates this check from vha to vp, and another follow-up commit ("scsi:
qla2xxx: Re-acquire vport_slock and refresh tvp during vport DPC") further
updates the loop.

> diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
> index 4ad23d206add2..f7a08bc0093a5 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) {

[Severity: High]
Should this check vp->flags.delete_progress instead of
vha->flags.delete_progress?

In drivers/scsi/qla2xxx/qla_mid.c:qla2x00_do_dpc_all_vps(), vha is always the
base physical host:

	if (vha->vp_idx)
		return;

Each virtual port on ha->vp_list is represented by the loop iterator vp, and
drivers/scsi/qla2xxx/qla_attr.c:qla24xx_vport_delete() only sets
flags.delete_progress on the virtual port being deleted, never on the base
physical host. Checking vha->flags.delete_progress here causes the condition
to remain true for deleting virtual ports, allowing qla2x00_do_dpc_vp(vp) to
continue running concurrently with teardown and potentially causing a
use-after-free or null pointer dereference.

Although the follow-up commit "scsi: qla2xxx: Fix use-after-free races during
vport deletion" changes this to check !vp->flags.delete_progress, this commit
is broken on its own and is tagged for stable.

Additionally, even when checking vp->flags.delete_progress, is delete_progress
set early enough in drivers/scsi/qla2xxx/qla_attr.c:qla24xx_vport_delete() to
prevent races with qla2x00_do_dpc_vp()?

In drivers/scsi/qla2xxx/qla_attr.c:qla24xx_vport_delete():

	set_bit(VPORT_DELETE, &vha->dpc_flags);

	while (test_bit(LOOP_RESYNC_ACTIVE, &vha->dpc_flags))
		msleep(1000);


	qla24xx_disable_vp(vha);
	qla2x00_wait_for_sess_deletion(vha);

	qla_nvme_delete(vha);
	qla_enode_stop(vha);
	qla_edb_stop(vha);

	vha->flags.delete_progress = 1;

	qlt_remove_target(ha, vha);

	fc_remove_host(vha->host);

	scsi_remove_host(vha->host);

	/* Allow timer to run to drain queued items, when removing vp */
	qla24xx_deallocate_vp_id(vha);

Before flags.delete_progress is set, qla24xx_vport_delete() has already
waited for resync, disabled the vport, deleted sessions, and stopped
nvme/enode/edb. Could qla2x00_do_dpc_vp(vp) still enter during that window to
start a new loop resync, configure the vport, process purex items, or queue
relogin work, and then continue executing concurrently with
qlt_remove_target(), fc_remove_host(), and scsi_remove_host() before
qla24xx_deallocate_vp_id() waits for vref_count to drain?

Would it be safer to check test_bit(VPORT_DELETE, &vp->dpc_flags) here, matching
drivers/scsi/qla2xxx/qla_mid.c:qla2x00_alert_all_vps()?

>  			atomic_inc(&vp->vref_count);
>  			spin_unlock_irqrestore(&ha->vport_slock, flags);
>

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

[PATCH 08/14] 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 (the base vport), log a fatal
message and return early instead of proceeding with deletion logic. This
avoids unexpected behavior during vport teardown and ensures proper error
handling.

    [ 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..e6fc47c3f87f 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 0;
+	}
+
 	set_bit(VPORT_DELETE, &vha->dpc_flags);
 
 	while (test_bit(LOOP_RESYNC_ACTIVE, &vha->dpc_flags))
-- 
2.52.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: qla2xxx: qla24xx_vport_delete() returns 0 on invalid vp_idx error
--

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

scsi: qla2xxx: Add validation check for base vport index during delete

This commit adds a check in qla24xx_vport_delete() to log an error and return
early if vha->vp_idx is zero.

> diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
> index a4ca22024edeb..e6fc47c3f87f8 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 0;
> +	}

[Severity: High]
Should qla24xx_vport_delete() in drivers/scsi/qla2xxx/qla_attr.c return a
non-zero error code here instead of 0 when !vha->vp_idx?

qla24xx_vport_delete() is the .vport_delete callback invoked by
fc_vport_terminate() in drivers/scsi/scsi_transport_fc.c:

	if (i->f->vport_delete)
		stat = i->f->vport_delete(vport);
	else
		stat = -ENOENT;

	spin_lock_irqsave(shost->host_lock, flags);
	vport->flags &= ~FC_VPORT_DELETING;
	if (!stat) {
		vport->flags |= FC_VPORT_DELETED;
		list_del(&vport->peers);
		fc_host->npiv_vports_inuse--;
		scsi_host_put(shost);		/* for fc_host->vport list */
	}
	spin_unlock_irqrestore(shost->host_lock, flags);

	if (stat)
		return stat;
	...
	device_del(dev);
	transport_destroy_device(dev);

	put_device(dev);			/* for self-reference */

When qla24xx_vport_delete() returns 0, fc_vport_terminate() treats the
deletion as successful and unregisters and frees struct fc_vport, even though
qla24xx_vport_delete() skipped tearing down vha, removing vha->host, stopping
the vport timer, freeing vha->gnl.l and vha->scan.l, removing vha from
ha->vp_list in qla24xx_deallocate_vp_id(), and decrementing
ha->cur_vport_count.

Does this leave vha active on ha->vp_list with a dangling vha->fc_vport
pointer, and cause qla2x00_delete_all_vps() in drivers/scsi/qla2xxx/qla_os.c
to loop infinitely on the freed vha->fc_vport during driver unload:

	while (ha->cur_vport_count) {
		spin_lock_irqsave(&ha->vport_slock, flags);

		BUG_ON(base_vha->list.next == &ha->vp_list);
		/* This assumes first entry in ha->vp_list is always base vha */
		vha = list_first_entry(&base_vha->list, scsi_qla_host_t, list);
		scsi_host_get(vha->host);

		spin_unlock_irqrestore(&ha->vport_slock, flags);
		mutex_unlock(&ha->vport_lock);

		qla_nvme_delete(vha);

		fc_vport_terminate(vha->fc_vport);
		scsi_host_put(vha->host);

		mutex_lock(&ha->vport_lock);
	}

because vha is never removed from base_vha->list and ha->cur_vport_count is
never decremented?

> +
>  	set_bit(VPORT_DELETE, &vha->dpc_flags);
>  
>  	while (test_bit(LOOP_RESYNC_ACTIVE, &vha->dpc_flags))

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

[PATCH 09/14] 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 10/14] 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.

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  |  9 +++++----
 drivers/scsi/qla2xxx/qla_os.c   | 30 ++++++++++++++++-------------
 3 files changed, 43 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..40b8cbdece7d 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,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 && !vha->flags.delete_progress) {
+		if (vp->vp_idx && !vp->flags.delete_progress) {
 			atomic_inc(&vp->vref_count);
 			spin_unlock_irqrestore(&ha->vport_slock, flags);
 
@@ -447,7 +448,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

Re: [PATCH 10/14] 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: stale tvp and list_del_init() loop in vp_list traversals
- [Medium] scsi: qla2xxx: missing vport delete_progress check in qla_init.c
--

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

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

This commit checks vp_idx and delete_progress before referencing vports in
several ha->vp_list traversals and switches qla24xx_deallocate_vp_id() from
list_del() to list_del_init(). A subsequent commit in the series refreshes
tvp after re-locking vport_slock in qla2x00_do_dpc_all_vps() and
qla2x00_alert_all_vps(), but leaves other ha->vp_list traversals unfixed.

> 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);

[ ... ]

> @@ -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);

[Severity: Medium]
Should the ha->vp_list traversals in qla2x00_quiesce_io() and
qla82xx_restart_isp() in drivers/scsi/qla2xxx/qla_init.c also check
vp->vp_idx and !vp->flags.delete_progress before taking a reference and
dropping ha->vport_slock?

In drivers/scsi/qla2xxx/qla_init.c, qla2x00_quiesce_io() (called from
qla2x00_do_dpc(), qla82xx_need_qsnt_handler(), and
qla8044_need_qsnt_handler()) drops ha->vport_slock without checking either
vp->vp_idx or !vp->flags.delete_progress:

	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() (called from qla82xx_error_recovery(),
qla82xx_fcoe_ctx_reset(), and qla8044_isp_reset()) checks vp->vp_idx, but
does not check !vp->flags.delete_progress before dropping ha->vport_slock:

	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);

If fc_vport_terminate() -> qla24xx_vport_delete() runs concurrently on a
vport after setting vha->flags.delete_progress = 1 (and after calling
qla24xx_disable_vp(), qlt_remove_target(), fc_remove_host(), and
scsi_remove_host()), couldn't qla2x00_quiesce_io() and
qla82xx_restart_isp() still operate on the deleting vport (including
re-enabling it via qla2x00_vp_abort_isp() -> qla24xx_enable_vp()) or
dereference a freed tvp when ha->vport_slock is dropped on a preceding list
entry?

> diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
> index 90f09d36705a6..40b8cbdece7dd 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 switching to list_del_init(&vha->list) in qla24xx_deallocate_vp_id()
still cause a use-after-free or an infinite loop with ha->vport_slock held
and interrupts disabled when racing with list_for_each_entry_safe(vp, tvp,
&ha->vp_list, list)?

In qla2x00_abort_isp_cleanup() and qla2x00_abort_isp() in
drivers/scsi/qla2xxx/qla_init.c, qla_pci_error_cleanup() in
drivers/scsi/qla2xxx/qla_os.c, and qla2x00_do_dpc_all_vps() and
qla2x00_alert_all_vps() in drivers/scsi/qla2xxx/qla_mid.c,
list_for_each_entry_safe() prefetches tvp = list_next_entry(vp, list)
before entering the loop body for vp. Only vp->vref_count is incremented
before spin_unlock_irqrestore(&ha->vport_slock, flags) drops the lock; no
reference is held on tvp.

While ha->vport_slock is dropped while processing vp (for example in
qla2x00_mark_all_devices_lost(vp) or qla2x00_vp_abort_isp(vp)), another
thread running qla24xx_vport_delete() in drivers/scsi/qla2xxx/qla_attr.c on
tvp can set tvp->flags.delete_progress = 1 and call
qla24xx_deallocate_vp_id(tvp):

	/* Allow timer to run to drain queued items, when removing vp */
	qla24xx_deallocate_vp_id(vha);
	...
	ql_log(ql_log_info, vha, 0x7088, "VP[%d] deleted.\n", id);
	scsi_host_put(vha->host);

Because atomic_read(&tvp->vref_count) is 0, qla24xx_deallocate_vp_id(tvp)
acquires ha->vport_slock, executes list_del_init(&tvp->list) (setting
tvp->list.next = &tvp->list), and returns, allowing qla24xx_vport_delete()
to free tvp via scsi_host_put(tvp->host).

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

1. It dereferences tvp (reading tvp->list.next, vp->vp_idx, and
   vp->flags.delete_progress) after tvp has been unlinked and potentially
   freed by scsi_host_put(tvp->host).

2. Because list_del_init(&tvp->list) set tvp->list.next = &tvp->list,
   list_next_entry(tvp, list) evaluates to tvp itself,
   !list_entry_is_head(vp, &ha->vp_list, list) remains permanently true,
   and the loop body skips unlocking ha->vport_slock because
   tvp->flags.delete_progress is 1. Doesn't this spin forever on the CPU
   with ha->vport_slock held and local interrupts disabled?

3. In addition, on the timeout path (!bailout) in
   qla24xx_deallocate_vp_id(), list_del_init(&vha->list) unlinks vha and
   qla24xx_vport_delete() proceeds to scsi_host_put(vha->host) even while
   vha->vref_count > 0 and a traversal thread is still actively using vha.

A later commit in the series ("scsi: qla2xxx: Re-acquire vport_slock and
refresh tvp during vport DPC") refreshes tvp = list_next_entry(vp, list)
after re-locking ha->vport_slock in qla2x00_do_dpc_all_vps() and
qla2x00_alert_all_vps(), but both loops in qla2x00_abort_isp_cleanup(), the
loop in qla2x00_abort_isp(), and both loops in qla_pci_error_cleanup() do
not refresh tvp after re-acquiring ha->vport_slock.

[ ... ]

> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 0047c2bd531b3..5593dff7eb9b7 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);

[ ... ]

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

[PATCH 11/14] 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 40b8cbdece7d..a2d76c8e81e5 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,
@@ -445,11 +450,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

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

[sashiko-bot]

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

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

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

This commit changes qla2x00_do_dpc_all_vps() to re-acquire vport_slock
using a spin_trylock_irqsave() and cond_resched() loop, and refreshes the
next list entry pointer tvp in qla2x00_do_dpc_all_vps() and
qla2x00_alert_all_vps() after re-locking. It also adds a cond_resched() call
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 40b8cbdece7dd..a2d76c8e81e5f 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,
> @@ -445,11 +450,28 @@ qla2x00_do_dpc_all_vps(scsi_qla_host_t *vha)
>  			spin_unlock_irqrestore(&ha->vport_slock, flags);
>  
>  			qla2x00_do_dpc_vp(vp);

[Severity: Medium]
In drivers/scsi/qla2xxx/qla_mid.c, qla2x00_do_dpc_all_vps() checks
!vp->flags.delete_progress before dropping ha->vport_slock and calling
qla2x00_do_dpc_vp(vp), but unlike qla2x00_alert_all_vps() it does not check
test_bit(VPORT_DELETE, &vp->dpc_flags).

In drivers/scsi/qla2xxx/qla_attr.c, qla24xx_vport_delete() sets VPORT_DELETE
and waits for LOOP_RESYNC_ACTIVE to clear, then performs several blocking
teardown calls before setting vha->flags.delete_progress = 1:

drivers/scsi/qla2xxx/qla_attr.c:qla24xx_vport_delete() {
	...
	set_bit(VPORT_DELETE, &vha->dpc_flags);

	while (test_bit(LOOP_RESYNC_ACTIVE, &vha->dpc_flags))
		msleep(1000);


	qla24xx_disable_vp(vha);
	qla2x00_wait_for_sess_deletion(vha);

	qla_nvme_delete(vha);
	qla_enode_stop(vha);
	qla_edb_stop(vha);

	vha->flags.delete_progress = 1;
	...
}

Should qla2x00_do_dpc_all_vps() (or qla2x00_do_dpc_vp()) also check
test_bit(VPORT_DELETE, &vp->dpc_flags) so that it cannot start a new
qla2x00_loop_resync(), qla24xx_configure_vp(), or qla24xx_post_relogin_work()
after qla24xx_vport_delete() has already passed the LOOP_RESYNC_ACTIVE wait?

> -
> -			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 this trigger a use-after-free on vp in qla2x00_do_dpc_all_vps() (and on
vha in qla2x00_alert_all_vps() above) when the deallocate timeout occurs?

When qla2x00_do_dpc_all_vps() or qla2x00_alert_all_vps() drops
ha->vport_slock, it only increments vref_count without taking a reference on
the underlying Scsi_Host via scsi_host_get().

If qla2x00_do_dpc_vp(vp) (which performs synchronous mailbox commands,
firmware readiness waits, loop resync, and cond_resched()) or
qla2x00_async_event() plus the spin_trylock_irqsave() loop takes longer than
the 10 second wait in qla24xx_deallocate_vp_id(), the timeout path unlinks
vha->list under ha->vport_slock 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(vha) returns,
qla24xx_vport_delete() in drivers/scsi/qla2xxx/qla_attr.c proceeds to tear
down vha and free the Scsi_Host containing vha:

drivers/scsi/qla2xxx/qla_attr.c:qla24xx_vport_delete() {
	...
	/* Allow timer to run to drain queued items, when removing vp */
	qla24xx_deallocate_vp_id(vha);
	...
	scsi_host_put(vha->host);
	return 0;
}

By the time qla2x00_do_dpc_all_vps() or qla2x00_alert_all_vps() re-acquires
ha->vport_slock in this timeout case, wouldn't vp (or vha) already be freed
when atomic_dec(&vp->vref_count), wake_up(&vp->vref_waitq), and
list_empty(&vp->list) access it?

> +			tvp = list_next_entry(vp, list);

[Severity: High]
Are the other list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) loops in
the driver that drop ha->vport_slock susceptible to the same stale tvp
infinite loop and use-after-free?

In drivers/scsi/qla2xxx/qla_init.c, qla2x00_quiesce_io(), both loops in
qla2x00_abort_isp_cleanup(), qla2x00_abort_isp(), and qla82xx_restart_isp()
all drop ha->vport_slock while holding only vp->vref_count, leaving
tvp->vref_count at zero without refreshing tvp after re-locking:

drivers/scsi/qla2xxx/qla_init.c:qla2x00_abort_isp_cleanup() {
	...
		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_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);
	...
}

The same pattern also appears in both vp_list loops in
qla_pci_error_cleanup() in drivers/scsi/qla2xxx/qla_os.c.

If a concurrent qla24xx_vport_delete() -> qla24xx_deallocate_vp_id() removes
and frees tvp via list_del_init(&tvp->list) and scsi_host_put(tvp->host)
while ha->vport_slock is dropped in those functions, won't the next loop
iteration dereference the freed tvp and spin forever on the self-referencing
list node with interrupts disabled?

>  		}
>  	}
>  	spin_unlock_irqrestore(&ha->vport_slock, flags);

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

[PATCH 12/14] scsi: qla2xxx: Yield work_lock in fabric scan path

[Nilesh Javali]

A hard lockup was observed in the fabric scan path after multiple
iterations of vport delete, with the work queue worker spinning on
work_lock in qla_fab_scan_finish() while another path held it.

 NMI watchdog: Watchdog detected hard LOCKUP on cpu 38
 Workqueue: qla2xxx_wq qla2x00_iocb_work_fn [qla2xxx]
 RIP: 0010:native_queued_spin_lock_slowpath+0x5f/0x1c0
 RSP: 0018:ffffb9a88faafd58 EFLAGS: 00000002
 RAX: 0000000000000101 RBX: 0000000000000202 RCX: 0000000000000800
 RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff98b149ff6488
 RBP: ffff98b803e60a58 R08: 000000000086742c R09: 0000000000000000
      R10: 2100000eddd08241 R11: 0000000000000000 R12: 000000000000c000
      R13: ffffb9a8a4555000 R14: 0000000000000000 R15: 2100000eddd08241
     FS:  0000000000000000(0000) GS:ffff98b7def80000(0000) knlGS:0000000000000000
     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
     CR2: 00007f5da15934c0 CR3: 000000033ee10000 CR4: 0000000000350ee0
     Call Trace:
      _raw_spin_lock_irqsave+0x34/0x40
      qla_fab_scan_finish+0x527/0x700 [qla2xxx]
      qla2x00_do_work+0x390/0x490 [qla2xxx]
      ? __switch_to_asm+0x43/0x80
      ? __switch_to+0x10c/0x450
      ? finish_task_switch+0x86/0x2e0
      qla2x00_iocb_work_fn+0x42/0x80 [qla2xxx]
      process_one_work+0x1a7/0x360
      ? create_worker+0x1a0/0x1a0

In the scan-finish cleanup path, replace the blocking
spin_lock_irqsave(&vha->work_lock) with a spin_trylock_irqsave() +
cond_resched() yield loop so the worker yields instead of spinning if the
lock is held, avoiding the hard lockup.

Cc: stable@vger.kernel.org
Fixes: f352eeb75419 ("scsi: qla2xxx: Add ability to use GPNFT/GNNFT for RSCN handling")
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_gs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index efe98609c149..31308a614858 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -3386,7 +3386,9 @@ void qla_fab_scan_finish(scsi_qla_host_t *vha, srb_t *sp)
 	recheck = 1;
 out:
 	qla24xx_sp_unmap(vha, sp);
-	spin_lock_irqsave(&vha->work_lock, flags);
+	/* Avoid hard lockup: yield if work_lock is held instead of spinning */
+	while (!spin_trylock_irqsave(&vha->work_lock, flags))
+		cond_resched();
 	vha->scan.scan_flags &= ~SF_SCANNING;
 	spin_unlock_irqrestore(&vha->work_lock, flags);
 
-- 
2.52.0

Re: [PATCH 12/14] scsi: qla2xxx: Yield work_lock in fabric scan path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: qla2xxx: wrong lock targeted with trylock loop in qla_fab_scan_finish()
--

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

scsi: qla2xxx: Yield work_lock in fabric scan path

This commit replaces spin_lock_irqsave(&vha->work_lock, flags) in
qla_fab_scan_finish() with a spin_trylock_irqsave() and cond_resched() loop
to avoid a hard lockup observed during vport deletion.

> diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
> index efe98609c149d..31308a6148585 100644
> --- a/drivers/scsi/qla2xxx/qla_gs.c
> +++ b/drivers/scsi/qla2xxx/qla_gs.c
> @@ -3386,7 +3386,9 @@ void qla_fab_scan_finish(scsi_qla_host_t *vha, srb_t *sp)
>  	recheck = 1;
>  out:
>  	qla24xx_sp_unmap(vha, sp);
> -	spin_lock_irqsave(&vha->work_lock, flags);
> +	/* Avoid hard lockup: yield if work_lock is held instead of spinning */
> +	while (!spin_trylock_irqsave(&vha->work_lock, flags))
> +		cond_resched();
>  	vha->scan.scan_flags &= ~SF_SCANNING;
>  	spin_unlock_irqrestore(&vha->work_lock, flags);
>  

[Severity: High]
Is this targeting the right spinlock for the hard lockup shown in the commit
message, and is a trylock loop safe here?

In the qla2x00_iocb_work_fn() -> qla2x00_do_work() -> qla_fab_scan_finish()
path, the static helper qla2x00_is_a_vp() is inlined into the
max_fibre_devices scan loop in qla_fab_scan_finish():

drivers/scsi/qla2xxx/qla_gs.c:qla2x00_is_a_vp() {
	...
	spin_lock_irqsave(&ha->vport_slock, flags);
	list_for_each_entry(vp, &ha->vp_list, list) {
		twwn = wwn_to_u64(vp->port_name);
		if (wwn == twwn) {
			rc = 1;
			break;
		}
	}
	spin_unlock_irqrestore(&ha->vport_slock, flags);
	...
}

drivers/scsi/qla2xxx/qla_gs.c:qla_fab_scan_finish() {
	...
	for (i = 0; i < vha->hw->max_fibre_devices; i++) {
		...
		/* Bypass virtual ports of the same host. */
		if (qla2x00_is_a_vp(vha, wwn))
			continue;
		...
	}
}

Looking at the registers in the backtrace:
- rbp is 0xffff98b803e60a58, which is vha (at the shost_priv offset 0xa58),
  where &vha->work_lock at offset 0x30 would be 0xffff98b803e60a88.
- rdi (the lock pointer passed to _raw_spin_lock_irqsave) is
  0xffff98b149ff6488, which is at offset 0x488 of struct qla_hw_data *ha
  (0xffff98b149ff6000), matching &ha->vport_slock (as also seen in commits
  79b305e1e477, 0334585318b2, and 200e63c70fc1).
- r10 and r15 hold 0x2100000eddd08241 (the 64-bit port name wwn passed to
  qla2x00_is_a_vp(vha, wwn)), r13 holds 0xffffb9a8a4555000 (vha->scan.l),
  rcx holds 0x800 (vha->hw->max_fibre_devices), and r12 holds 0xc000
  (2048 * sizeof(struct fab_scan_rp)).

This indicates the worker was spinning on &ha->vport_slock inside
qla2x00_is_a_vp() rather than on &vha->work_lock at the out label.

In addition, vha->work_lock is not taken in the vport delete path
(qla24xx_vport_delete() -> qla24xx_disable_vp() ->
qla24xx_deallocate_vp_id()) and is only held across short, non-blocking flag
and list updates with interrupts disabled. By contrast, ha->vport_slock
protects ha->vp_list during vport deletion, where several
list_for_each_entry_safe(vp, tvp, &ha->vp_list, list) loops (such as in
qla2x00_abort_isp_cleanup(), qla2x00_abort_isp(), qla2x00_quiesce_io(),
qla82xx_restart_isp(), and qla_pci_error_cleanup()) drop and re-acquire
ha->vport_slock without refreshing tvp if qla24xx_deallocate_vp_id() removes
an entry via list_del_init(&vha->list).

Could replacing spin_lock_irqsave(&vha->work_lock, flags) with a
spin_trylock_irqsave() + cond_resched() loop also introduce regressions by
bypassing queued spinlock fairness and realtime priority inheritance,
busy-looping without cpu_relax() while toggling interrupts when
need_resched() is false, or livelocking the qla2xxx_wq worker if the lock is
ever stuck (leaving scan_flags with SF_SCANNING set and hanging
flush_workqueue(vha->hw->wq) in qla2x00_wait_for_sess_deletion())?

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

[PATCH 13/14] 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 14/14] 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