scsi: qedi: Fix race during abort timeouts

BugLink: https://bugs.launchpad.net/bugs/1939738
[ Upstream commit 2ce002366a3fcc3f9616d4583194f65dde0ad253 ]

If the SCSI cmd completes after qedi_tmf_work calls iscsi_itt_to_task then
the qedi qedi_cmd->task_id could be freed and used for another cmd. If we
then call qedi_iscsi_cleanup_task with that task_id we will be cleaning up
the wrong cmd.

Wait to release the task_id until the last put has been done on the
iscsi_task. Because libiscsi grabs a ref to the task when sending the
abort, we know that for the non-abort timeout case that the task_id we are
referencing is for the cmd that was supposed to be aborted.

A latter commit will fix the case where the abort times out while we are
running qedi_tmf_work.

Link: https://lore.kernel.org/r/20210525181821.7617-21-michael.christie@oracle.com
Reviewed-by: Manish Rangankar <mrangankar@marvell.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>

diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
index cf57b4e49700fd25869632b3afc577d800eaea4f..c12bb2dd5ff92b864b8e3e7e578a7ae2dc990707 100644 (file)
--- a/drivers/scsi/qedi/qedi_fw.c
+++ b/drivers/scsi/qedi/qedi_fw.c
@@ -73,7 +73,6 @@ static void qedi_process_logout_resp(struct qedi_ctx *qedi,
        spin_unlock(&qedi_conn->list_lock);
 
        cmd->state = RESPONSE_RECEIVED;
-       qedi_clear_task_idx(qedi, cmd->task_id);
        __iscsi_complete_pdu(conn, (struct iscsi_hdr *)resp_hdr, NULL, 0);
 
        spin_unlock(&session->back_lock);
@@ -138,7 +137,6 @@ static void qedi_process_text_resp(struct qedi_ctx *qedi,
        spin_unlock(&qedi_conn->list_lock);
 
        cmd->state = RESPONSE_RECEIVED;
-       qedi_clear_task_idx(qedi, cmd->task_id);
 
        __iscsi_complete_pdu(conn, (struct iscsi_hdr *)resp_hdr_ptr,
                             qedi_conn->gen_pdu.resp_buf,
@@ -164,13 +162,11 @@ static void qedi_tmf_resp_work(struct work_struct *work)
        iscsi_block_session(session->cls_session);
        rval = qedi_cleanup_all_io(qedi, qedi_conn, qedi_cmd->task, true);
        if (rval) {
-               qedi_clear_task_idx(qedi, qedi_cmd->task_id);
                iscsi_unblock_session(session->cls_session);
                goto exit_tmf_resp;
        }
 
        iscsi_unblock_session(session->cls_session);
-       qedi_clear_task_idx(qedi, qedi_cmd->task_id);
 
        spin_lock(&session->back_lock);
        __iscsi_complete_pdu(conn, (struct iscsi_hdr *)resp_hdr_ptr, NULL, 0);
@@ -245,8 +241,6 @@ static void qedi_process_tmf_resp(struct qedi_ctx *qedi,
                goto unblock_sess;
        }
 
-       qedi_clear_task_idx(qedi, qedi_cmd->task_id);
-
        __iscsi_complete_pdu(conn, (struct iscsi_hdr *)resp_hdr_ptr, NULL, 0);
        kfree(resp_hdr_ptr);
 
@@ -314,7 +308,6 @@ static void qedi_process_login_resp(struct qedi_ctx *qedi,
                  "Freeing tid=0x%x for cid=0x%x\n",
                  cmd->task_id, qedi_conn->iscsi_conn_id);
        cmd->state = RESPONSE_RECEIVED;
-       qedi_clear_task_idx(qedi, cmd->task_id);
 }
 
 static void qedi_get_rq_bdq_buf(struct qedi_ctx *qedi,
@@ -468,7 +461,6 @@ static int qedi_process_nopin_mesg(struct qedi_ctx *qedi,
                }
 
                spin_unlock(&qedi_conn->list_lock);
-               qedi_clear_task_idx(qedi, cmd->task_id);
        }
 
 done:
@@ -673,7 +665,6 @@ static void qedi_scsi_completion(struct qedi_ctx *qedi,
        if (qedi_io_tracing)
                qedi_trace_io(qedi, task, cmd->task_id, QEDI_IO_TRACE_RSP);
 
-       qedi_clear_task_idx(qedi, cmd->task_id);
        __iscsi_complete_pdu(conn, (struct iscsi_hdr *)hdr,
                             conn->data, datalen);
 error:
@@ -730,7 +721,6 @@ static void qedi_process_nopin_local_cmpl(struct qedi_ctx *qedi,
                  cqe->itid, cmd->task_id);
 
        cmd->state = RESPONSE_RECEIVED;
-       qedi_clear_task_idx(qedi, cmd->task_id);
 
        spin_lock_bh(&session->back_lock);
        __iscsi_put_task(task);
@@ -748,7 +738,6 @@ static void qedi_process_cmd_cleanup_resp(struct qedi_ctx *qedi,
        itt_t protoitt = 0;
        int found = 0;
        struct qedi_cmd *qedi_cmd = NULL;
-       u32 rtid = 0;
        u32 iscsi_cid;
        struct qedi_conn *qedi_conn;
        struct qedi_cmd *dbg_cmd;
@@ -779,7 +768,6 @@ static void qedi_process_cmd_cleanup_resp(struct qedi_ctx *qedi,
                        found = 1;
                        mtask = qedi_cmd->task;
                        tmf_hdr = (struct iscsi_tm *)mtask->hdr;
-                       rtid = work->rtid;
 
                        list_del_init(&work->list);
                        kfree(work);
@@ -821,8 +809,6 @@ static void qedi_process_cmd_cleanup_resp(struct qedi_ctx *qedi,
                        if (qedi_cmd->state == CLEANUP_WAIT_FAILED)
                                qedi_cmd->state = CLEANUP_RECV;
 
-                       qedi_clear_task_idx(qedi_conn->qedi, rtid);
-
                        spin_lock(&qedi_conn->list_lock);
                        if (likely(dbg_cmd->io_cmd_in_list)) {
                                dbg_cmd->io_cmd_in_list = false;
@@ -856,7 +842,6 @@ static void qedi_process_cmd_cleanup_resp(struct qedi_ctx *qedi,
                QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_TID,
                          "Freeing tid=0x%x for cid=0x%x\n",
                          cqe->itid, qedi_conn->iscsi_conn_id);
-               qedi_clear_task_idx(qedi_conn->qedi, cqe->itid);
 
        } else {
                qedi_get_proto_itt(qedi, cqe->itid, &ptmp_itt);

diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index 087c7ff28cd5205325da8eecee9a7c8d4a3e2da4..4acc12111330ceacc77c6a7ce0c3a41ef8023aeb 100644 (file)
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -783,7 +783,6 @@ static int qedi_mtask_xmit(struct iscsi_conn *conn, struct iscsi_task *task)
        }
 
        cmd->conn = conn->dd_data;
-       cmd->scsi_cmd = NULL;
        return qedi_iscsi_send_generic_request(task);
 }
 
@@ -794,6 +793,10 @@ static int qedi_task_xmit(struct iscsi_task *task)
        struct qedi_cmd *cmd = task->dd_data;
        struct scsi_cmnd *sc = task->sc;
 
+       /* Clear now so in cleanup_task we know it didn't make it */
+       cmd->scsi_cmd = NULL;
+       cmd->task_id = U16_MAX;
+
        if (test_bit(QEDI_IN_SHUTDOWN, &qedi_conn->qedi->flags))
                return -ENODEV;
 
@@ -1394,13 +1397,24 @@ static umode_t qedi_attr_is_visible(int param_type, int param)
 
 static void qedi_cleanup_task(struct iscsi_task *task)
 {
-       if (!task->sc || task->state == ISCSI_TASK_PENDING) {
+       struct qedi_cmd *cmd;
+
+       if (task->state == ISCSI_TASK_PENDING) {
                QEDI_INFO(NULL, QEDI_LOG_IO, "Returning ref_cnt=%d\n",
                          refcount_read(&task->refcount));
                return;
        }
 
-       qedi_iscsi_unmap_sg_list(task->dd_data);
+       if (task->sc)
+               qedi_iscsi_unmap_sg_list(task->dd_data);
+
+       cmd = task->dd_data;
+       if (cmd->task_id != U16_MAX)
+               qedi_clear_task_idx(iscsi_host_priv(task->conn->session->host),
+                                   cmd->task_id);
+
+       cmd->task_id = U16_MAX;
+       cmd->scsi_cmd = NULL;
 }
 
 struct iscsi_transport qedi_iscsi_transport = {