From: Nicholas Bellinger nab@linux-iscsi.org
Hi Greg-KH,
Here are two target patches for v4.9.y stable, the first of which did not originally include a stable CC, and the latter did not apply due to a minor context change.
The series has been cut against v4.9.62. Please apply at your earliest convenience.
Thank you,
--nab
Bart Van Assche (1): target/iscsi: Fix iSCSI task reassignment handling
Nicholas Bellinger (1): qla2xxx: Fix incorrect tcm_qla2xxx_free_cmd use during TMR ABORT (v2)
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 33 --------------------------------- drivers/target/iscsi/iscsi_target.c | 19 +++++++------------ include/target/target_core_base.h | 1 + 3 files changed, 8 insertions(+), 45 deletions(-)
From: Bart Van Assche bart.vanassche@sandisk.com
commit 59b6986dbfcdab96a971f9663221849de79a7556 upstream.
Allocate a task management request structure for all task management requests, including task reassignment. This change avoids that the se_tmr->response assignment dereferences an uninitialized se_tmr pointer.
Reported-by: Moshe David mdavid@infinidat.com Signed-off-by: Bart Van Assche bart.vanassche@sandisk.com Reviewed-by: Hannes Reinecke hare@suse.com Reviewed-by: Christoph Hellwig hch@lst.de Cc: Moshe David mdavid@infinidat.com Signed-off-by: Nicholas Bellinger nab@linux-iscsi.org --- drivers/target/iscsi/iscsi_target.c | 19 +++++++------------ include/target/target_core_base.h | 1 + 2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index e49fcd5..f3c9d18 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -1940,7 +1940,7 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd, struct iscsi_tm *hdr; int out_of_order_cmdsn = 0, ret; bool sess_ref = false; - u8 function; + u8 function, tcm_function = TMR_UNKNOWN;
hdr = (struct iscsi_tm *) buf; hdr->flags &= ~ISCSI_FLAG_CMD_FINAL; @@ -1986,10 +1986,6 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd, * LIO-Target $FABRIC_MOD */ if (function != ISCSI_TM_FUNC_TASK_REASSIGN) { - - u8 tcm_function; - int ret; - transport_init_se_cmd(&cmd->se_cmd, &iscsi_ops, conn->sess->se_sess, 0, DMA_NONE, TCM_SIMPLE_TAG, cmd->sense_buffer + 2); @@ -2025,15 +2021,14 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd, return iscsit_add_reject_cmd(cmd, ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf); } - - ret = core_tmr_alloc_req(&cmd->se_cmd, cmd->tmr_req, - tcm_function, GFP_KERNEL); - if (ret < 0) - return iscsit_add_reject_cmd(cmd, + } + ret = core_tmr_alloc_req(&cmd->se_cmd, cmd->tmr_req, tcm_function, + GFP_KERNEL); + if (ret < 0) + return iscsit_add_reject_cmd(cmd, ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
- cmd->tmr_req->se_tmr_req = cmd->se_cmd.se_tmr_req; - } + cmd->tmr_req->se_tmr_req = cmd->se_cmd.se_tmr_req;
cmd->iscsi_opcode = ISCSI_OP_SCSI_TMFUNC; cmd->i_state = ISTATE_SEND_TASKMGTRSP; diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 0383c60..a87e894 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -197,6 +197,7 @@ enum tcm_tmreq_table { TMR_LUN_RESET = 5, TMR_TARGET_WARM_RESET = 6, TMR_TARGET_COLD_RESET = 7, + TMR_UNKNOWN = 0xff, };
/* fabric independent task management response values */
From: Nicholas Bellinger nab@linux-iscsi.org
commit 6bcbb3174caa5f1ccc894f8ae077631659d5a629 upstream.
This patch drops two incorrect usages of tcm_qla2xxx_free_cmd() during TMR ABORT within tcm_qla2xxx_handle_data_work() and tcm_qla2xxx_aborted_task(), which where attempting to dispatch into workqueue context to do tcm_qla2xxx_complete_free() and subsequently invoke transport_generic_free_cmd().
This is incorrect because during TMR ABORT target-core will drop the outstanding se_cmd->cmd_kref references once it has quiesced the se_cmd via transport_wait_for_tasks(), and in the case of qla2xxx it should not attempt to do it's own transport_generic_free_cmd() once the abort has occured.
As reported by Pascal, this was originally manifesting as a BUG_ON(cmd->cmd_in_wq) in qlt_free_cmd() during TMR ABORT, with a LIO backend that had sufficently high enough WRITE latency to trigger a host side TMR ABORT_TASK.
(v2: Drop the qla_tgt_cmd->write_pending_abort_comp changes, as they will be addressed in a seperate series)
Reported-by: Pascal de Bruijn p.debruijn@unilogic.nl Tested-by: Pascal de Bruijn p.debruijn@unilogic.nl Cc: Pascal de Bruijn p.debruijn@unilogic.nl Reported-by: Lukasz Engel lukasz.engel@softax.pl Cc: Lukasz Engel lukasz.engel@softax.pl Acked-by: Himanshu Madhani himanshu.madhani@cavium.com Cc: Quinn Tran quinn.tran@cavium.com Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger nab@linux-iscsi.org --- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 33 --------------------------------- 1 file changed, 33 deletions(-)
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index 6643f6f..0ad8ece 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -484,7 +484,6 @@ static int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd, static void tcm_qla2xxx_handle_data_work(struct work_struct *work) { struct qla_tgt_cmd *cmd = container_of(work, struct qla_tgt_cmd, work); - unsigned long flags;
/* * Ensure that the complete FCP WRITE payload has been received. @@ -492,17 +491,6 @@ static void tcm_qla2xxx_handle_data_work(struct work_struct *work) */ cmd->cmd_in_wq = 0;
- spin_lock_irqsave(&cmd->cmd_lock, flags); - cmd->cmd_flags |= CMD_FLAG_DATA_WORK; - if (cmd->aborted) { - cmd->cmd_flags |= CMD_FLAG_DATA_WORK_FREE; - spin_unlock_irqrestore(&cmd->cmd_lock, flags); - - tcm_qla2xxx_free_cmd(cmd); - return; - } - spin_unlock_irqrestore(&cmd->cmd_lock, flags); - cmd->vha->tgt_counters.qla_core_ret_ctio++; if (!cmd->write_data_transferred) { /* @@ -682,34 +670,13 @@ static void tcm_qla2xxx_queue_tm_rsp(struct se_cmd *se_cmd) qlt_xmit_tm_rsp(mcmd); }
- -#define DATA_WORK_NOT_FREE(_flags) \ - (( _flags & (CMD_FLAG_DATA_WORK|CMD_FLAG_DATA_WORK_FREE)) == \ - CMD_FLAG_DATA_WORK) static void tcm_qla2xxx_aborted_task(struct se_cmd *se_cmd) { struct qla_tgt_cmd *cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd); - unsigned long flags;
if (qlt_abort_cmd(cmd)) return; - - spin_lock_irqsave(&cmd->cmd_lock, flags); - if ((cmd->state == QLA_TGT_STATE_NEW)|| - ((cmd->state == QLA_TGT_STATE_DATA_IN) && - DATA_WORK_NOT_FREE(cmd->cmd_flags)) ) { - - cmd->cmd_flags |= CMD_FLAG_DATA_WORK_FREE; - spin_unlock_irqrestore(&cmd->cmd_lock, flags); - /* Cmd have not reached firmware. - * Use this trigger to free it. */ - tcm_qla2xxx_free_cmd(cmd); - return; - } - spin_unlock_irqrestore(&cmd->cmd_lock, flags); - return; - }
static void tcm_qla2xxx_clear_sess_lookup(struct tcm_qla2xxx_lport *,
On Thu, Nov 16, 2017 at 06:05:07AM +0000, Nicholas A. Bellinger wrote:
From: Nicholas Bellinger nab@linux-iscsi.org
commit 6bcbb3174caa5f1ccc894f8ae077631659d5a629 upstream.
This patch drops two incorrect usages of tcm_qla2xxx_free_cmd() during TMR ABORT within tcm_qla2xxx_handle_data_work() and tcm_qla2xxx_aborted_task(), which where attempting to dispatch into workqueue context to do tcm_qla2xxx_complete_free() and subsequently invoke transport_generic_free_cmd().
This is incorrect because during TMR ABORT target-core will drop the outstanding se_cmd->cmd_kref references once it has quiesced the se_cmd via transport_wait_for_tasks(), and in the case of qla2xxx it should not attempt to do it's own transport_generic_free_cmd() once the abort has occured.
As reported by Pascal, this was originally manifesting as a BUG_ON(cmd->cmd_in_wq) in qlt_free_cmd() during TMR ABORT, with a LIO backend that had sufficently high enough WRITE latency to trigger a host side TMR ABORT_TASK.
(v2: Drop the qla_tgt_cmd->write_pending_abort_comp changes, as they will be addressed in a seperate series)
Reported-by: Pascal de Bruijn p.debruijn@unilogic.nl Tested-by: Pascal de Bruijn p.debruijn@unilogic.nl Cc: Pascal de Bruijn p.debruijn@unilogic.nl Reported-by: Lukasz Engel lukasz.engel@softax.pl Cc: Lukasz Engel lukasz.engel@softax.pl Acked-by: Himanshu Madhani himanshu.madhani@cavium.com Cc: Quinn Tran quinn.tran@cavium.com Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger nab@linux-iscsi.org
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 33 --------------------------------- 1 file changed, 33 deletions(-)
Thanks for this, but how about a backport for it for 4.4 and 3.18?
thanks,
greg k-h
On Thu, Nov 16, 2017 at 06:05:05AM +0000, Nicholas A. Bellinger wrote:
From: Nicholas Bellinger nab@linux-iscsi.org
Hi Greg-KH,
Here are two target patches for v4.9.y stable, the first of which did not originally include a stable CC, and the latter did not apply due to a minor context change.
The series has been cut against v4.9.62. Please apply at your earliest convenience.
All applied, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg-KH -
Nicholas A. Bellinger