| From ecebaab729c1bea8a36a56cff2ddf7a54bddb6e5 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 25 May 2021 13:18:13 -0500 |
| Subject: scsi: qedi: Fix race during abort timeouts |
| |
| From: Mike Christie <michael.christie@oracle.com> |
| |
| [ 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> |
| --- |
| drivers/scsi/qedi/qedi_fw.c | 15 --------------- |
| drivers/scsi/qedi/qedi_iscsi.c | 20 +++++++++++++++++--- |
| 2 files changed, 17 insertions(+), 18 deletions(-) |
| |
| diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c |
| index cf57b4e49700..c12bb2dd5ff9 100644 |
| --- 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 087c7ff28cd5..4acc12111330 100644 |
| --- 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 = { |
| -- |
| 2.30.2 |
| |