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>

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index f0e02ad83ac02e31afedb422e87a03ea5640e57d..34ee7a179a798e73d76a63353a7c59c8def0c4b4 100644 (file)
--- 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 849ab256807be87100f34e6e94a00f281b86b040..009b9ca5c2b945fcb2aad843de8893e168c2fb73 100644 (file)
--- 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,21 @@ 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)
+               return;
+       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 +2318,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 +2356,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 +5734,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 +5745,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 eb15d8e9f79ebb63239b86e7494e4ced1811018a..223c40bc94982c81e3150565f70383712a067c2e 100644 (file)
--- 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);