Skip to content

Commit 3d56983

Browse files
abattersbymartinkpetersen
authored andcommitted
scsi: qla2xxx: Fix TMR failure handling
(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> Link: https://patch.msgid.link/09a1ff3d-6738-4953-a31b-10e89c540462@cybernetics.com Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 5c50d84 commit 3d56983

3 files changed

Lines changed: 27 additions & 33 deletions

File tree

drivers/scsi/qla2xxx/qla_os.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1893,7 +1893,7 @@ __qla2x00_abort_all_cmds(struct qla_qpair *qp, int res)
18931893
* Currently, only ABTS response gets on the
18941894
* outstanding_cmds[]
18951895
*/
1896-
ha->tgt.tgt_ops->free_mcmd(
1896+
qlt_free_ul_mcmd(ha,
18971897
(struct qla_tgt_mgmt_cmd *) sp);
18981898
break;
18991899
default:

drivers/scsi/qla2xxx/qla_target.c

Lines changed: 24 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2005,7 +2005,6 @@ static void qlt_do_tmr_work(struct work_struct *work)
20052005
struct qla_hw_data *ha = mcmd->vha->hw;
20062006
int rc;
20072007
uint32_t tag;
2008-
unsigned long flags;
20092008

20102009
switch (mcmd->tmr_func) {
20112010
case QLA_TGT_ABTS:
@@ -2020,34 +2019,12 @@ static void qlt_do_tmr_work(struct work_struct *work)
20202019
mcmd->tmr_func, tag);
20212020

20222021
if (rc != 0) {
2023-
spin_lock_irqsave(mcmd->qpair->qp_lock_ptr, flags);
2024-
switch (mcmd->tmr_func) {
2025-
case QLA_TGT_ABTS:
2026-
mcmd->fc_tm_rsp = FCP_TMF_REJECTED;
2027-
qlt_build_abts_resp_iocb(mcmd);
2028-
break;
2029-
case QLA_TGT_LUN_RESET:
2030-
case QLA_TGT_CLEAR_TS:
2031-
case QLA_TGT_ABORT_TS:
2032-
case QLA_TGT_CLEAR_ACA:
2033-
case QLA_TGT_TARGET_RESET:
2034-
qlt_send_busy(mcmd->qpair, &mcmd->orig_iocb.atio,
2035-
qla_sam_status);
2036-
break;
2037-
2038-
case QLA_TGT_ABORT_ALL:
2039-
case QLA_TGT_NEXUS_LOSS_SESS:
2040-
case QLA_TGT_NEXUS_LOSS:
2041-
qlt_send_notify_ack(mcmd->qpair,
2042-
&mcmd->orig_iocb.imm_ntfy, 0, 0, 0, 0, 0, 0);
2043-
break;
2044-
}
2045-
spin_unlock_irqrestore(mcmd->qpair->qp_lock_ptr, flags);
2046-
20472022
ql_dbg(ql_dbg_tgt_mgt, mcmd->vha, 0xf052,
20482023
"qla_target(%d): tgt_ops->handle_tmr() failed: %d\n",
20492024
mcmd->vha->vp_idx, rc);
2050-
mempool_free(mcmd, qla_tgt_mgmt_cmd_mempool);
2025+
mcmd->flags |= QLA24XX_MGMT_LLD_OWNED;
2026+
mcmd->fc_tm_rsp = FCP_TMF_FAILED;
2027+
qlt_xmit_tm_rsp(mcmd);
20512028
}
20522029
}
20532030

@@ -2234,6 +2211,21 @@ void qlt_free_mcmd(struct qla_tgt_mgmt_cmd *mcmd)
22342211
}
22352212
EXPORT_SYMBOL(qlt_free_mcmd);
22362213

2214+
/*
2215+
* If the upper layer knows about this mgmt cmd, then call its ->free_cmd()
2216+
* callback, which will eventually call qlt_free_mcmd(). Otherwise, call
2217+
* qlt_free_mcmd() directly.
2218+
*/
2219+
void qlt_free_ul_mcmd(struct qla_hw_data *ha, struct qla_tgt_mgmt_cmd *mcmd)
2220+
{
2221+
if (!mcmd)
2222+
return;
2223+
if (mcmd->flags & QLA24XX_MGMT_LLD_OWNED)
2224+
qlt_free_mcmd(mcmd);
2225+
else
2226+
ha->tgt.tgt_ops->free_mcmd(mcmd);
2227+
}
2228+
22372229
/*
22382230
* ha->hardware_lock supposed to be held on entry. Might drop it, then
22392231
* reacquire
@@ -2326,12 +2318,12 @@ void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *mcmd)
23262318
"RESET-TMR online/active/old-count/new-count = %d/%d/%d/%d.\n",
23272319
vha->flags.online, qla2x00_reset_active(vha),
23282320
mcmd->reset_count, qpair->chip_reset);
2329-
ha->tgt.tgt_ops->free_mcmd(mcmd);
2321+
qlt_free_ul_mcmd(ha, mcmd);
23302322
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
23312323
return;
23322324
}
23332325

2334-
if (mcmd->flags == QLA24XX_MGMT_SEND_NACK) {
2326+
if (mcmd->flags & QLA24XX_MGMT_SEND_NACK) {
23352327
switch (mcmd->orig_iocb.imm_ntfy.u.isp24.status_subcode) {
23362328
case ELS_LOGO:
23372329
case ELS_PRLO:
@@ -2364,7 +2356,7 @@ void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *mcmd)
23642356
* qlt_xmit_tm_rsp() returns here..
23652357
*/
23662358
if (free_mcmd)
2367-
ha->tgt.tgt_ops->free_mcmd(mcmd);
2359+
qlt_free_ul_mcmd(ha, mcmd);
23682360

23692361
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
23702362
}
@@ -5742,7 +5734,7 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
57425734
if (le32_to_cpu(entry->error_subcode1) == 0x1E &&
57435735
le32_to_cpu(entry->error_subcode2) == 0) {
57445736
if (qlt_chk_unresolv_exchg(vha, rsp->qpair, entry)) {
5745-
ha->tgt.tgt_ops->free_mcmd(mcmd);
5737+
qlt_free_ul_mcmd(ha, mcmd);
57465738
return;
57475739
}
57485740
qlt_24xx_retry_term_exchange(vha, rsp->qpair,
@@ -5753,10 +5745,10 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
57535745
vha->vp_idx, entry->compl_status,
57545746
entry->error_subcode1,
57555747
entry->error_subcode2);
5756-
ha->tgt.tgt_ops->free_mcmd(mcmd);
5748+
qlt_free_ul_mcmd(ha, mcmd);
57575749
}
57585750
} else if (mcmd) {
5759-
ha->tgt.tgt_ops->free_mcmd(mcmd);
5751+
qlt_free_ul_mcmd(ha, mcmd);
57605752
}
57615753
}
57625754

drivers/scsi/qla2xxx/qla_target.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,7 @@ struct qla_tgt_mgmt_cmd {
966966
unsigned int flags;
967967
#define QLA24XX_MGMT_SEND_NACK BIT_0
968968
#define QLA24XX_MGMT_ABORT_IO_ATTR_VALID BIT_1
969+
#define QLA24XX_MGMT_LLD_OWNED BIT_2
969970
uint32_t reset_count;
970971
struct work_struct work;
971972
uint64_t unpacked_lun;
@@ -1059,6 +1060,7 @@ extern int qlt_abort_cmd(struct qla_tgt_cmd *);
10591060
void qlt_send_term_exchange(struct qla_qpair *qpair,
10601061
struct qla_tgt_cmd *cmd, struct atio_from_isp *atio, int ha_locked);
10611062
extern void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *);
1063+
void qlt_free_ul_mcmd(struct qla_hw_data *ha, struct qla_tgt_mgmt_cmd *mcmd);
10621064
extern void qlt_free_mcmd(struct qla_tgt_mgmt_cmd *);
10631065
extern void qlt_free_cmd(struct qla_tgt_cmd *cmd);
10641066
extern void qlt_unmap_sg(struct scsi_qla_host *vha, struct qla_tgt_cmd *cmd);

0 commit comments

Comments
 (0)