Commit 39c0c855 authored by Bart Van Assche's avatar Bart Van Assche Committed by Martin K. Petersen

scsi: Revert "qla2xxx: Make sure that aborted commands are freed"

Calling vha->hw->tgt.tgt_ops->free_cmd() from qlt_xmit_response() is wrong
since the command for which a response is sent must remain valid until the
SCSI target core calls .release_cmd(). It has been observed that the
following scenario triggers a kernel crash:

 - qlt_xmit_response() calls qlt_check_reserve_free_req()

 - qlt_check_reserve_free_req() returns -EAGAIN

 - qlt_xmit_response() calls vha->hw->tgt.tgt_ops->free_cmd(cmd)

 - transport_handle_queue_full() tries to retransmit the response

Fix this crash by reverting the patch that introduced it.

Link: https://lore.kernel.org/r/20210320232359.941-2-bvanassche@acm.org
Fixes: 0dcec41a ("scsi: qla2xxx: Make sure that aborted commands are freed")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: default avatarDaniel Wagner <dwagner@suse.de>
Reviewed-by: default avatarHimanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 62fc2661
...@@ -3222,8 +3222,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type, ...@@ -3222,8 +3222,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
if (!qpair->fw_started || (cmd->reset_count != qpair->chip_reset) || if (!qpair->fw_started || (cmd->reset_count != qpair->chip_reset) ||
(cmd->sess && cmd->sess->deleted)) { (cmd->sess && cmd->sess->deleted)) {
cmd->state = QLA_TGT_STATE_PROCESSED; cmd->state = QLA_TGT_STATE_PROCESSED;
res = 0; return 0;
goto free;
} }
ql_dbg_qp(ql_dbg_tgt, qpair, 0xe018, ql_dbg_qp(ql_dbg_tgt, qpair, 0xe018,
...@@ -3234,8 +3233,9 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type, ...@@ -3234,8 +3233,9 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
res = qlt_pre_xmit_response(cmd, &prm, xmit_type, scsi_status, res = qlt_pre_xmit_response(cmd, &prm, xmit_type, scsi_status,
&full_req_cnt); &full_req_cnt);
if (unlikely(res != 0)) if (unlikely(res != 0)) {
goto free; return res;
}
spin_lock_irqsave(qpair->qp_lock_ptr, flags); spin_lock_irqsave(qpair->qp_lock_ptr, flags);
...@@ -3255,8 +3255,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type, ...@@ -3255,8 +3255,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
vha->flags.online, qla2x00_reset_active(vha), vha->flags.online, qla2x00_reset_active(vha),
cmd->reset_count, qpair->chip_reset); cmd->reset_count, qpair->chip_reset);
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags); spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
res = 0; return 0;
goto free;
} }
/* Does F/W have an IOCBs for this request */ /* Does F/W have an IOCBs for this request */
...@@ -3359,8 +3358,6 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type, ...@@ -3359,8 +3358,6 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
qlt_unmap_sg(vha, cmd); qlt_unmap_sg(vha, cmd);
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags); spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
free:
vha->hw->tgt.tgt_ops->free_cmd(cmd);
return res; return res;
} }
EXPORT_SYMBOL(qlt_xmit_response); EXPORT_SYMBOL(qlt_xmit_response);
......
...@@ -644,7 +644,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd) ...@@ -644,7 +644,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
{ {
struct qla_tgt_cmd *cmd = container_of(se_cmd, struct qla_tgt_cmd *cmd = container_of(se_cmd,
struct qla_tgt_cmd, se_cmd); struct qla_tgt_cmd, se_cmd);
struct scsi_qla_host *vha = cmd->vha;
if (cmd->aborted) { if (cmd->aborted) {
/* Cmd can loop during Q-full. tcm_qla2xxx_aborted_task /* Cmd can loop during Q-full. tcm_qla2xxx_aborted_task
...@@ -657,7 +656,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd) ...@@ -657,7 +656,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
cmd->se_cmd.transport_state, cmd->se_cmd.transport_state,
cmd->se_cmd.t_state, cmd->se_cmd.t_state,
cmd->se_cmd.se_cmd_flags); cmd->se_cmd.se_cmd_flags);
vha->hw->tgt.tgt_ops->free_cmd(cmd);
return 0; return 0;
} }
...@@ -685,7 +683,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd) ...@@ -685,7 +683,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
{ {
struct qla_tgt_cmd *cmd = container_of(se_cmd, struct qla_tgt_cmd *cmd = container_of(se_cmd,
struct qla_tgt_cmd, se_cmd); struct qla_tgt_cmd, se_cmd);
struct scsi_qla_host *vha = cmd->vha;
int xmit_type = QLA_TGT_XMIT_STATUS; int xmit_type = QLA_TGT_XMIT_STATUS;
if (cmd->aborted) { if (cmd->aborted) {
...@@ -699,7 +696,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd) ...@@ -699,7 +696,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
cmd, kref_read(&cmd->se_cmd.cmd_kref), cmd, kref_read(&cmd->se_cmd.cmd_kref),
cmd->se_cmd.transport_state, cmd->se_cmd.t_state, cmd->se_cmd.transport_state, cmd->se_cmd.t_state,
cmd->se_cmd.se_cmd_flags); cmd->se_cmd.se_cmd_flags);
vha->hw->tgt.tgt_ops->free_cmd(cmd);
return 0; return 0;
} }
cmd->bufflen = se_cmd->data_length; cmd->bufflen = se_cmd->data_length;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment