From: Tony Battersby <tonyb@cybernetics.com>
To: Nilesh Javali <njavali@marvell.com>,
GR-QLogic-Storage-Upstream@marvell.com,
"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
target-devel@vger.kernel.org, scst-devel@lists.sourceforge.net,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Dmitry Bogdanov <d.bogdanov@yadro.com>,
Xose Vazquez Perez <xose.vazquez@gmail.com>
Subject: [PATCH v2 11/16] scsi: qla2xxx: fix TMR failure handling
Date: Mon, 29 Sep 2025 10:43:28 -0400 [thread overview]
Message-ID: <f52cda16-4952-4b28-bbf7-d44f4e054490@cybernetics.com> (raw)
In-Reply-To: <e95ee7d0-3580-4124-b854-7f73ca3a3a84@cybernetics.com>
(target mode)
If handle_tmr() fails:
- The code for QLA_TGT_ABTS results in memory-use-after-free and
double-free:
qlt_do_tmr_work()
qlt_build_abts_resp_iocb()
qpair->req->outstanding_cmds[h] = (srb_t *)mcmd;
mempool_free(mcmd, qla_tgt_mgmt_cmd_mempool); FIRST FREE
qlt_handle_abts_completion()
mcmd = qlt_ctio_to_cmd()
cmd = req->outstanding_cmds[h];
return cmd;
vha = mcmd->vha; USE-AFTER-FREE
ha->tgt.tgt_ops->free_mcmd(mcmd); SECOND FREE
- qlt_send_busy() makes no sense because it sends a SCSI command
response instead of a TMR response.
Instead just call qlt_xmit_tm_rsp() to send a TMR failed response,
since that code is well-tested and handles a number of corner cases.
But it would be incorrect to call ha->tgt.tgt_ops->free_mcmd() after
handle_tmr() failed, so add a flag to mcmd indicating the proper way to
free the mcmd so that qlt_xmit_tm_rsp() can be used for both cases.
Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
---
v1 -> v2:
- Change FCP_TMF_REJECTED to FCP_TMF_FAILED.
- Add QLA24XX_MGMT_LLD_OWNED and qlt_free_ul_mcmd().
- Improve patch description.
drivers/scsi/qla2xxx/qla_os.c | 2 +-
drivers/scsi/qla2xxx/qla_target.c | 54 +++++++++++++------------------
drivers/scsi/qla2xxx/qla_target.h | 2 ++
3 files changed, 25 insertions(+), 33 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 2a3eb1dacf86..64387224f28a 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1893,7 +1893,7 @@ __qla2x00_abort_all_cmds(struct qla_qpair *qp, int res)
* Currently, only ABTS response gets on the
* outstanding_cmds[]
*/
- ha->tgt.tgt_ops->free_mcmd(
+ qlt_free_ul_mcmd(ha,
(struct qla_tgt_mgmt_cmd *) sp);
break;
default:
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 849ab256807b..69ccba3436ec 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -2005,7 +2005,6 @@ static void qlt_do_tmr_work(struct work_struct *work)
struct qla_hw_data *ha = mcmd->vha->hw;
int rc;
uint32_t tag;
- unsigned long flags;
switch (mcmd->tmr_func) {
case QLA_TGT_ABTS:
@@ -2020,34 +2019,12 @@ static void qlt_do_tmr_work(struct work_struct *work)
mcmd->tmr_func, tag);
if (rc != 0) {
- spin_lock_irqsave(mcmd->qpair->qp_lock_ptr, flags);
- switch (mcmd->tmr_func) {
- case QLA_TGT_ABTS:
- mcmd->fc_tm_rsp = FCP_TMF_REJECTED;
- qlt_build_abts_resp_iocb(mcmd);
- break;
- case QLA_TGT_LUN_RESET:
- case QLA_TGT_CLEAR_TS:
- case QLA_TGT_ABORT_TS:
- case QLA_TGT_CLEAR_ACA:
- case QLA_TGT_TARGET_RESET:
- qlt_send_busy(mcmd->qpair, &mcmd->orig_iocb.atio,
- qla_sam_status);
- break;
-
- case QLA_TGT_ABORT_ALL:
- case QLA_TGT_NEXUS_LOSS_SESS:
- case QLA_TGT_NEXUS_LOSS:
- qlt_send_notify_ack(mcmd->qpair,
- &mcmd->orig_iocb.imm_ntfy, 0, 0, 0, 0, 0, 0);
- break;
- }
- spin_unlock_irqrestore(mcmd->qpair->qp_lock_ptr, flags);
-
ql_dbg(ql_dbg_tgt_mgt, mcmd->vha, 0xf052,
"qla_target(%d): tgt_ops->handle_tmr() failed: %d\n",
mcmd->vha->vp_idx, rc);
- mempool_free(mcmd, qla_tgt_mgmt_cmd_mempool);
+ mcmd->flags |= QLA24XX_MGMT_LLD_OWNED;
+ mcmd->fc_tm_rsp = FCP_TMF_FAILED;
+ qlt_xmit_tm_rsp(mcmd);
}
}
@@ -2234,6 +2211,19 @@ void qlt_free_mcmd(struct qla_tgt_mgmt_cmd *mcmd)
}
EXPORT_SYMBOL(qlt_free_mcmd);
+/*
+ * If the upper layer knows about this mgmt cmd, then call its ->free_cmd()
+ * callback, which will eventually call qlt_free_mcmd(). Otherwise, call
+ * qlt_free_mcmd() directly.
+ */
+void qlt_free_ul_mcmd(struct qla_hw_data *ha, struct qla_tgt_mgmt_cmd *mcmd)
+{
+ if (mcmd->flags & QLA24XX_MGMT_LLD_OWNED)
+ qlt_free_mcmd(mcmd);
+ else
+ ha->tgt.tgt_ops->free_mcmd(mcmd);
+}
+
/*
* ha->hardware_lock supposed to be held on entry. Might drop it, then
* reacquire
@@ -2326,12 +2316,12 @@ void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *mcmd)
"RESET-TMR online/active/old-count/new-count = %d/%d/%d/%d.\n",
vha->flags.online, qla2x00_reset_active(vha),
mcmd->reset_count, qpair->chip_reset);
- ha->tgt.tgt_ops->free_mcmd(mcmd);
+ qlt_free_ul_mcmd(ha, mcmd);
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
return;
}
- if (mcmd->flags == QLA24XX_MGMT_SEND_NACK) {
+ if (mcmd->flags & QLA24XX_MGMT_SEND_NACK) {
switch (mcmd->orig_iocb.imm_ntfy.u.isp24.status_subcode) {
case ELS_LOGO:
case ELS_PRLO:
@@ -2364,7 +2354,7 @@ void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *mcmd)
* qlt_xmit_tm_rsp() returns here..
*/
if (free_mcmd)
- ha->tgt.tgt_ops->free_mcmd(mcmd);
+ qlt_free_ul_mcmd(ha, mcmd);
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
}
@@ -5742,7 +5732,7 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
if (le32_to_cpu(entry->error_subcode1) == 0x1E &&
le32_to_cpu(entry->error_subcode2) == 0) {
if (qlt_chk_unresolv_exchg(vha, rsp->qpair, entry)) {
- ha->tgt.tgt_ops->free_mcmd(mcmd);
+ qlt_free_ul_mcmd(ha, mcmd);
return;
}
qlt_24xx_retry_term_exchange(vha, rsp->qpair,
@@ -5753,10 +5743,10 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
vha->vp_idx, entry->compl_status,
entry->error_subcode1,
entry->error_subcode2);
- ha->tgt.tgt_ops->free_mcmd(mcmd);
+ qlt_free_ul_mcmd(ha, mcmd);
}
} else if (mcmd) {
- ha->tgt.tgt_ops->free_mcmd(mcmd);
+ qlt_free_ul_mcmd(ha, mcmd);
}
}
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index eb15d8e9f79e..223c40bc9498 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -966,6 +966,7 @@ struct qla_tgt_mgmt_cmd {
unsigned int flags;
#define QLA24XX_MGMT_SEND_NACK BIT_0
#define QLA24XX_MGMT_ABORT_IO_ATTR_VALID BIT_1
+#define QLA24XX_MGMT_LLD_OWNED BIT_2
uint32_t reset_count;
struct work_struct work;
uint64_t unpacked_lun;
@@ -1059,6 +1060,7 @@ extern int qlt_abort_cmd(struct qla_tgt_cmd *);
void qlt_send_term_exchange(struct qla_qpair *qpair,
struct qla_tgt_cmd *cmd, struct atio_from_isp *atio, int ha_locked);
extern void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *);
+void qlt_free_ul_mcmd(struct qla_hw_data *ha, struct qla_tgt_mgmt_cmd *mcmd);
extern void qlt_free_mcmd(struct qla_tgt_mgmt_cmd *);
extern void qlt_free_cmd(struct qla_tgt_cmd *cmd);
extern void qlt_unmap_sg(struct scsi_qla_host *vha, struct qla_tgt_cmd *cmd);
--
2.43.0
next prev parent reply other threads:[~2025-09-29 14:43 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-29 14:28 [PATCH v2 00/16] qla2xxx target mode improvements Tony Battersby
2025-09-29 14:30 ` [PATCH v2 01/16] Revert "scsi: qla2xxx: Perform lockless command completion in abort path" Tony Battersby
2025-09-29 14:32 ` [PATCH v2 02/16] scsi: qla2xxx: fix initiator mode with qlini_mode=exclusive Tony Battersby
2025-09-29 14:33 ` [PATCH v2 03/16] scsi: qla2xxx: fix lost interrupts with qlini_mode=disabled Tony Battersby
2025-09-29 14:34 ` [PATCH v2 04/16] scsi: qla2xxx: use reinit_completion on mbx_intr_comp Tony Battersby
2025-09-29 14:35 ` [PATCH v2 05/16] scsi: qla2xxx: remove code for unsupported hardware Tony Battersby
2025-09-29 14:36 ` [PATCH v2 06/16] scsi: qla2xxx: improve debug output for term exchange Tony Battersby
2025-09-29 14:37 ` [PATCH v2 07/16] scsi: qla2xxx: fix term exchange when cmd_sent_to_fw == 1 Tony Battersby
2025-09-29 14:38 ` [PATCH v2 08/16] scsi: qla2xxx: clear cmds after chip reset Tony Battersby
2025-09-29 14:39 ` [PATCH v2 09/16] scsi: qla2xxx: fix races with aborting commands Tony Battersby
2025-09-29 14:41 ` [SCST PATCH " Tony Battersby
2025-09-29 14:42 ` [PATCH v2 10/16] scsi: qla2xxx: improve checks in qlt_xmit_response / qlt_rdy_to_xfer Tony Battersby
2025-09-29 14:43 ` Tony Battersby [this message]
2025-10-03 8:40 ` [PATCH v2 11/16] scsi: qla2xxx: fix TMR failure handling Dan Carpenter
2025-10-03 14:38 ` [PATCH v3 " Tony Battersby
2025-09-29 14:44 ` [PATCH v2 12/16] scsi: qla2xxx: fix invalid memory access with big CDBs Tony Battersby
2025-09-29 14:45 ` [SCST PATCH " Tony Battersby
2025-09-29 14:47 ` [PATCH v2 13/16] scsi: qla2xxx: add cmd->rsp_sent Tony Battersby
2025-09-29 14:48 ` [SCST PATCH " Tony Battersby
2025-09-29 14:49 ` [PATCH v2 14/16] scsi: qla2xxx: improve cmd logging Tony Battersby
2025-09-29 14:50 ` [PATCH v2 15/16] scsi: qla2xxx: add back SRR support Tony Battersby
2025-09-29 14:51 ` [SCST PATCH " Tony Battersby
2025-09-29 14:53 ` [PATCH v2 16/16] scsi: qla2xxx: improve safety of cmd lookup by handle Tony Battersby
2025-09-29 14:54 ` [SCST PATCH v2] qla2x00t-32gbit: add on_abort_cmd callback Tony Battersby
2025-11-03 15:44 ` [PATCH v2 00/16] qla2xxx target mode improvements Tony Battersby
2025-11-08 17:03 ` Martin K. Petersen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f52cda16-4952-4b28-bbf7-d44f4e054490@cybernetics.com \
--to=tonyb@cybernetics.com \
--cc=GR-QLogic-Storage-Upstream@marvell.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=d.bogdanov@yadro.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=njavali@marvell.com \
--cc=scst-devel@lists.sourceforge.net \
--cc=target-devel@vger.kernel.org \
--cc=xose.vazquez@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox