From 18a9df42d53fabfa43b78be1104838cc8b9762e1 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Mon, 17 Dec 2012 09:53:32 +0100 Subject: [PATCH 1/9] target: use correct sense code for LUN communication failure The ASC/ASCQ code for 'Logical Unit Communication failure' is 0x08/0x00; 0x80/0x00 is vendor specific. Signed-off-by: Hannes Reinecke Cc: Nicholas Bellinger Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index c23c76ccef65..978762dc81bf 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2743,7 +2743,7 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd, /* ILLEGAL REQUEST */ buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST; /* LOGICAL UNIT COMMUNICATION FAILURE */ - buffer[SPC_ASC_KEY_OFFSET] = 0x80; + buffer[SPC_ASC_KEY_OFFSET] = 0x08; break; } /* From ba829137bfd167623363548aa385be769c6b2664 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Mon, 17 Dec 2012 09:53:33 +0100 Subject: [PATCH 2/9] target: Introduce TCM_NO_SENSE Introduce TCM_NO_SENSE, mapping to sense code 'Not ready, no additional sense information'. Signed-off-by: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 10 ++++++++++ include/target/target_core_base.h | 1 + 2 files changed, 11 insertions(+) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 978762dc81bf..104bf3b45a01 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2597,6 +2597,16 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd, * SENSE KEY values from include/scsi/scsi.h */ switch (reason) { + case TCM_NO_SENSE: + /* CURRENT ERROR */ + buffer[0] = 0x70; + buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10; + /* Not Ready */ + buffer[SPC_SENSE_KEY_OFFSET] = NOT_READY; + /* NO ADDITIONAL SENSE INFORMATION */ + buffer[SPC_ASC_KEY_OFFSET] = 0; + buffer[SPC_ASCQ_KEY_OFFSET] = 0; + break; case TCM_NON_EXISTENT_LUN: /* CURRENT ERROR */ buffer[0] = 0x70; diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 7cae2360221e..663e34a5383f 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -174,6 +174,7 @@ typedef unsigned __bitwise__ sense_reason_t; enum tcm_sense_reason_table { #define R(x) (__force sense_reason_t )(x) + TCM_NO_SENSE = R(0x00), TCM_NON_EXISTENT_LUN = R(0x01), TCM_UNSUPPORTED_SCSI_OPCODE = R(0x02), TCM_INCORRECT_AMOUNT_OF_DATA = R(0x03), From a0d50f62c8b7e461f9d53124576d59bc582f3eb3 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Mon, 17 Dec 2012 09:53:34 +0100 Subject: [PATCH 3/9] target: Use TCM_NO_SENSE for initialisation The compiler complained about uninitialized variables, so use TCM_NO_SENSE here. Signed-off-by: Hannes Reinecke Cc: Nicholas Bellinger Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_alua.c | 2 +- drivers/target/target_core_pr.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c index 85140f7dde1e..7d4ec02e29a9 100644 --- a/drivers/target/target_core_alua.c +++ b/drivers/target/target_core_alua.c @@ -212,7 +212,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd) struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem, *l_tg_pt_gp_mem; unsigned char *buf; unsigned char *ptr; - sense_reason_t rc; + sense_reason_t rc = TCM_NO_SENSE; u32 len = 4; /* Skip over RESERVED area in header */ int alua_access_state, primary = 0; u16 tg_pt_id, rtpi; diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index e35dbf85841f..8e0290b38e43 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -2053,7 +2053,7 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key, /* Used for APTPL metadata w/ UNREGISTER */ unsigned char *pr_aptpl_buf = NULL; unsigned char isid_buf[PR_REG_ISID_LEN], *isid_ptr = NULL; - sense_reason_t ret; + sense_reason_t ret = TCM_NO_SENSE; int pr_holder = 0, type; if (!se_sess || !se_lun) { From f2eeba214bcd0215b7f558cab6420e5fd153042b Mon Sep 17 00:00:00 2001 From: Mark Rustad Date: Fri, 21 Dec 2012 10:58:14 -0800 Subject: [PATCH 4/9] tcm_fc: Do not indicate retry capability to initiators When generating a PRLI response to an initiator, clear the FCP_SPPF_RETRY bit in the response. Signed-off-by: Mark Rustad Reviewed-by: Bhanu Prakash Gollapudi Acked by Robert Love Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/tcm_fc/tfc_sess.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c index 12d6fa21e5e1..59f2b467f9f7 100644 --- a/drivers/target/tcm_fc/tfc_sess.c +++ b/drivers/target/tcm_fc/tfc_sess.c @@ -396,10 +396,10 @@ static int ft_prli_locked(struct fc_rport_priv *rdata, u32 spp_len, /* * OR in our service parameters with other provider (initiator), if any. - * TBD XXX - indicate RETRY capability? */ fill: fcp_parm = ntohl(spp->spp_params); + fcp_parm &= ~FCP_SPPF_RETRY; spp->spp_params = htonl(fcp_parm | FCP_SPPF_TARG_FCN); return FC_SPP_RESP_ACK; } From edec8dfefa1f372b2dd8197da555352e76a10c03 Mon Sep 17 00:00:00 2001 From: Mark Rustad Date: Fri, 21 Dec 2012 10:58:19 -0800 Subject: [PATCH 5/9] tcm_fc: Do not report target role when target is not defined Clear the target role when no target is provided for the node performing a PRLI. Signed-off-by: Mark Rustad Reviewed-by: Bhanu Prakash Gollapudi Acked by Robert Love Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/tcm_fc/tfc_sess.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c index 59f2b467f9f7..6659dd36e806 100644 --- a/drivers/target/tcm_fc/tfc_sess.c +++ b/drivers/target/tcm_fc/tfc_sess.c @@ -355,11 +355,11 @@ static int ft_prli_locked(struct fc_rport_priv *rdata, u32 spp_len, tport = ft_tport_create(rdata->local_port); if (!tport) - return 0; /* not a target for this local port */ + goto not_target; /* not a target for this local port */ acl = ft_acl_get(tport->tpg, rdata); if (!acl) - return 0; + goto not_target; /* no target for this remote */ if (!rspp) goto fill; @@ -402,6 +402,12 @@ fill: fcp_parm &= ~FCP_SPPF_RETRY; spp->spp_params = htonl(fcp_parm | FCP_SPPF_TARG_FCN); return FC_SPP_RESP_ACK; + +not_target: + fcp_parm = ntohl(spp->spp_params); + fcp_parm &= ~FCP_SPPF_TARG_FCN; + spp->spp_params = htonl(fcp_parm); + return 0; } /** From e627c615553a356f6f70215ebb3933c6e057553e Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Wed, 2 Jan 2013 12:47:57 -0800 Subject: [PATCH 6/9] target: Fix missing CMD_T_ACTIVE bit regression for pending WRITEs This patch fixes a regression bug introduced during v3.6.x code with the following commit to drop transport_add_cmd_to_queue(), which originally re-set CMD_T_ACTIVE during pending WRITE I/O submission: commit af8772926f019b7bddd7477b8de5f3b0f12bad21 Author: Christoph Hellwig Date: Sun Jul 8 15:58:49 2012 -0400 target: replace the processing thread with a TMR work queue The following sequence happens for write commands (or any other commands with a data out phase): - The transport calls target_submit_cmd(), which sets CMD_T_ACTIVE in cmd->transport_state and sets cmd->t_state to TRANSPORT_NEW_CMD. - Things go on transport_generic_new_cmd(), which notices that the command needs to transfer data, so it sets cmd->t_state to TRANSPORT_WRITE_PENDING and calls transport_cmd_check_stop(). - transport_cmd_check_stop() clears CMD_T_ACTIVE in cmd->transport_state and returns in the normal case. - Then we continue on to call ->se_tfo->write_pending(). - The data comes back from the initiator, and the transport calls target_execute_cmd(), which sets cmd->t_state to TRANSPORT_PROCESSING and calls into the backend to actually write the data. At this point, the backend might take a long time to complete the command, since it has to do real IO. If an abort request comes in for this command at this point, it will not wait for the command to finish since CMD_T_ACTIVE is not set. Then when the command does finally finish, we blow up with use-after-free. Avoid this by setting CMD_T_ACTIVE in target_execute_cmd() so that transport_wait_for_tasks() waits for the command to finish executing. This matches the behavior from before commit 1389533ef944 ("target: remove transport_generic_handle_data"), when data was signaled via transport_generic_handle_data(), which set CMD_T_ACTIVE because it called transport_add_cmd_to_queue(). Signed-off-by: Roland Dreier Reported-by: Martin Svec Cc: Christoph Hellwig Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 104bf3b45a01..1775bda6c43c 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1688,6 +1688,7 @@ void target_execute_cmd(struct se_cmd *cmd) } cmd->t_state = TRANSPORT_PROCESSING; + cmd->transport_state |= CMD_T_ACTIVE; spin_unlock_irq(&cmd->t_state_lock); if (!target_handle_task_attr(cmd)) From 72b59d6ee8adaa51f70377db0a1917ed489bead8 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Wed, 2 Jan 2013 12:47:58 -0800 Subject: [PATCH 7/9] target: Fix use-after-free in LUN RESET handling If a backend IO takes a really long then an initiator might abort a command, and then when it gives up on the abort, send a LUN reset too, all before we process any of the original command or the abort. (The abort will wait for the backend IO to complete too) When the backend IO final completes (or fails), the abort handling will proceed and queue up a "return aborted status" operation. Then, while that's still pending, the LUN reset might find the original command still on the LUN's list of commands and try to return aborted status again, which leads to a use-after free when the first se_tfo->queue_status call frees the command and then the second se_tfo->queue_status call runs. Fix this by removing a command from the LUN state_list when we first are about to queue aborted status; we shouldn't do anything LUN-related after we've started returning status, so this seems like the correct thing to do. Signed-off-by: Roland Dreier Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 1775bda6c43c..489bd1678d87 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -541,9 +541,6 @@ static void transport_lun_remove_cmd(struct se_cmd *cmd) void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) { - if (!(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) - transport_lun_remove_cmd(cmd); - if (transport_cmd_check_stop_to_fabric(cmd)) return; if (remove) @@ -2815,6 +2812,8 @@ void transport_send_task_abort(struct se_cmd *cmd) } cmd->scsi_status = SAM_STAT_TASK_ABORTED; + transport_lun_remove_cmd(cmd); + pr_debug("Setting SAM_STAT_TASK_ABORTED status for CDB: 0x%02x," " ITT: 0x%08x\n", cmd->t_task_cdb[0], cmd->se_tfo->get_task_tag(cmd)); From 5a3b6fc0092c5f8dee7820064ee54d2631d48573 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Wed, 2 Jan 2013 12:47:59 -0800 Subject: [PATCH 8/9] target: Release se_cmd when LUN lookup fails for TMR When transport_lookup_tmr_lun() fails and we return a task management response from target_complete_tmr_failure(), we need to call transport_cmd_check_stop_to_fabric() to release the last ref to the cmd after calling se_tfo->queue_tm_rsp(), or else we will never remove the failed TMR from the session command list (and we'll end up waiting forever when trying to tear down the session). (nab: Fix minor compile breakage) Signed-off-by: Roland Dreier Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 489bd1678d87..bd587b70661a 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1393,6 +1393,8 @@ static void target_complete_tmr_failure(struct work_struct *work) se_cmd->se_tmr_req->response = TMR_LUN_DOES_NOT_EXIST; se_cmd->se_tfo->queue_tm_rsp(se_cmd); + + transport_cmd_check_stop_to_fabric(se_cmd); } /** From 64fe4f4f181cc2fe97d4176bf6ee6e3725ae33ec Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Mon, 7 Jan 2013 11:45:16 -0800 Subject: [PATCH 9/9] iscsi-target: Fix CmdSN comparison (use cmd->cmd_sn instead of cmd->stat_sn) Commit 64c13330a389 ("iscsi-target: Fix bug in handling of ExpStatSN ACK during u32 wrap-around") introduced a bug where we compare the wrong SN against our ExpCmdSN. Reported-by: Ben Hutchings Signed-off-by: Roland Dreier Cc: stable@vger.kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_erl2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/target/iscsi/iscsi_target_erl2.c b/drivers/target/iscsi/iscsi_target_erl2.c index 9ac4c151eae4..ba6091bf93fc 100644 --- a/drivers/target/iscsi/iscsi_target_erl2.c +++ b/drivers/target/iscsi/iscsi_target_erl2.c @@ -372,7 +372,7 @@ int iscsit_prepare_cmds_for_realligance(struct iscsi_conn *conn) * made generic here. */ if (!(cmd->cmd_flags & ICF_OOO_CMDSN) && !cmd->immediate_cmd && - iscsi_sna_gte(cmd->stat_sn, conn->sess->exp_cmd_sn)) { + iscsi_sna_gte(cmd->cmd_sn, conn->sess->exp_cmd_sn)) { list_del(&cmd->i_conn_node); spin_unlock_bh(&conn->cmd_lock); iscsit_free_cmd(cmd);