From 391e2a6de9781e4906dd7e0b1cc097050bf43e11 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Sun, 23 Oct 2016 14:28:15 -0700 Subject: target: Don't BUG_ON during NodeACL dynamic -> explicit conversion After the v4.2+ RCU conversion to se_node_acl->lun_entry_hlist, a BUG_ON() was added in core_enable_device_list_for_node() to detect when the located orig->se_lun_acl contains an existing se_lun_acl pointer reference. However, this scenario can happen when a dynamically generated NodeACL is being converted to an explicit NodeACL, when the explicit NodeACL contains a different LUN mapping than the default provided by the WWN endpoint. So instead of triggering BUG_ON(), go ahead and fail instead following the original pre RCU conversion logic. Reported-by: Benjamin ESTRABAUD Cc: Benjamin ESTRABAUD Reviewed-by: Christoph Hellwig Cc: stable@vger.kernel.org # 4.2+ Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_device.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 1ebd13ef7bd3..26929c44d703 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -352,7 +352,15 @@ int core_enable_device_list_for_node( kfree(new); return -EINVAL; } - BUG_ON(orig->se_lun_acl != NULL); + if (orig->se_lun_acl != NULL) { + pr_warn_ratelimited("Detected existing explicit" + " se_lun_acl->se_lun_group reference for %s" + " mapped_lun: %llu, failing\n", + nacl->initiatorname, mapped_lun); + mutex_unlock(&nacl->lun_entry_mutex); + kfree(new); + return -EINVAL; + } rcu_assign_pointer(new->se_lun, lun); rcu_assign_pointer(new->se_lun_acl, lun_acl); -- cgit v1.2.3 From 0583c261e6325f392c1f7a1b9112e31298e1a4bd Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Mon, 31 Oct 2016 00:54:40 -0700 Subject: target: Use correct SCSI status during EXTENDED_COPY exception This patch adds the missing target_complete_cmd() SCSI status parameter change in target_xcopy_do_work(), that was originally missing in commit 926317de33. It correctly propigates up the correct SCSI status during EXTENDED_COPY exception cases, instead of always using the hardcoded SAM_STAT_CHECK_CONDITION from original code. This is required for ESX host environments that expect to hit SAM_STAT_RESERVATION_CONFLICT for certain scenarios, and SAM_STAT_CHECK_CONDITION results in non-retriable status for these cases. Reported-by: Nixon Vincent Tested-by: Nixon Vincent Cc: Nixon Vincent Reviewed-by: Christoph Hellwig Cc: stable@vger.kernel.org # 3.14+ Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_xcopy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index d828b3b5000b..cac5a20a4de0 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -864,7 +864,7 @@ out: " CHECK_CONDITION -> sending response\n", rc); ec_cmd->scsi_status = SAM_STAT_CHECK_CONDITION; } - target_complete_cmd(ec_cmd, SAM_STAT_CHECK_CONDITION); + target_complete_cmd(ec_cmd, ec_cmd->scsi_status); } sense_reason_t target_do_xcopy(struct se_cmd *se_cmd) -- cgit v1.2.3 From c54eeffbe9338fa982dc853d816fda9202a13b5a Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Tue, 6 Dec 2016 22:45:46 -0800 Subject: target: Fix early transport_generic_handle_tmr abort scenario This patch fixes a bug where incoming task management requests can be explicitly aborted during an active LUN_RESET, but who's struct work_struct are canceled in-flight before execution. This occurs when core_tmr_drain_tmr_list() invokes cancel_work_sync() for the incoming se_tmr_req->task_cmd->work, resulting in cmd->work for target_tmr_work() never getting invoked and the aborted TMR waiting indefinately within transport_wait_for_tasks(). To address this case, perform a CMD_T_ABORTED check early in transport_generic_handle_tmr(), and invoke the normal path via transport_cmd_check_stop_to_fabric() to complete any TMR kthreads blocked waiting for CMD_T_STOP in transport_wait_for_tasks(). Also, move the TRANSPORT_ISTATE_PROCESSING assignment earlier into transport_generic_handle_tmr() so the existing check in core_tmr_drain_tmr_list() avoids attempting abort the incoming se_tmr_req->task_cmd->work if it has already been queued into se_device->tmr_wq. Reported-by: Rob Millner Tested-by: Rob Millner Cc: Rob Millner Reviewed-by: Christoph Hellwig Cc: stable@vger.kernel.org # 3.14+ Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 1cadc9eefa21..8b698432aea5 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3110,7 +3110,6 @@ static void target_tmr_work(struct work_struct *work) spin_unlock_irqrestore(&cmd->t_state_lock, flags); goto check_stop; } - cmd->t_state = TRANSPORT_ISTATE_PROCESSING; spin_unlock_irqrestore(&cmd->t_state_lock, flags); cmd->se_tfo->queue_tm_rsp(cmd); @@ -3123,11 +3122,25 @@ int transport_generic_handle_tmr( struct se_cmd *cmd) { unsigned long flags; + bool aborted = false; spin_lock_irqsave(&cmd->t_state_lock, flags); - cmd->transport_state |= CMD_T_ACTIVE; + if (cmd->transport_state & CMD_T_ABORTED) { + aborted = true; + } else { + cmd->t_state = TRANSPORT_ISTATE_PROCESSING; + cmd->transport_state |= CMD_T_ACTIVE; + } spin_unlock_irqrestore(&cmd->t_state_lock, flags); + if (aborted) { + pr_warn_ratelimited("handle_tmr caught CMD_T_ABORTED TMR %d" + "ref_tag: %llu tag: %llu\n", cmd->se_tmr_req->function, + cmd->se_tmr_req->ref_task_tag, cmd->tag); + transport_cmd_check_stop_to_fabric(cmd); + return 0; + } + INIT_WORK(&cmd->work, target_tmr_work); queue_work(cmd->se_dev->tmr_wq, &cmd->work); return 0; -- cgit v1.2.3 From 01d4d673558985d9a118e1e05026633c3e2ade9b Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Wed, 7 Dec 2016 12:55:54 -0800 Subject: target: Fix multi-session dynamic se_node_acl double free OOPs This patch addresses a long-standing bug with multi-session (eg: iscsi-target + iser-target) se_node_acl dynamic free withini transport_deregister_session(). This bug is caused when a storage endpoint is configured with demo-mode (generate_node_acls = 1 + cache_dynamic_acls = 1) initiators, and initiator login creates a new dynamic node acl and attaches two sessions to it. After that, demo-mode for the storage instance is disabled via configfs (generate_node_acls = 0 + cache_dynamic_acls = 0) and the existing dynamic acl is never converted to an explicit ACL. The end result is dynamic acl resources are released twice when the sessions are shutdown in transport_deregister_session(). If the storage instance is not changed to disable demo-mode, or the dynamic acl is converted to an explict ACL, or there is only a single session associated with the dynamic ACL, the bug is not triggered. To address this big, move the release of dynamic se_node_acl memory into target_complete_nacl() so it's only freed once when se_node_acl->acl_kref reaches zero. (Drop unnecessary list_del_init usage - HCH) Reported-by: Rob Millner Tested-by: Rob Millner Cc: Rob Millner Cc: stable@vger.kernel.org # 4.1+ Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 69 +++++++++++++++++++++------------- include/target/target_core_base.h | 1 + 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 8b698432aea5..437591bc7c08 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -457,8 +457,20 @@ static void target_complete_nacl(struct kref *kref) { struct se_node_acl *nacl = container_of(kref, struct se_node_acl, acl_kref); + struct se_portal_group *se_tpg = nacl->se_tpg; - complete(&nacl->acl_free_comp); + if (!nacl->dynamic_stop) { + complete(&nacl->acl_free_comp); + return; + } + + mutex_lock(&se_tpg->acl_node_mutex); + list_del(&nacl->acl_list); + mutex_unlock(&se_tpg->acl_node_mutex); + + core_tpg_wait_for_nacl_pr_ref(nacl); + core_free_device_list_for_node(nacl, se_tpg); + kfree(nacl); } void target_put_nacl(struct se_node_acl *nacl) @@ -499,12 +511,39 @@ EXPORT_SYMBOL(transport_deregister_session_configfs); void transport_free_session(struct se_session *se_sess) { struct se_node_acl *se_nacl = se_sess->se_node_acl; + /* * Drop the se_node_acl->nacl_kref obtained from within * core_tpg_get_initiator_node_acl(). */ if (se_nacl) { + struct se_portal_group *se_tpg = se_nacl->se_tpg; + const struct target_core_fabric_ops *se_tfo = se_tpg->se_tpg_tfo; + unsigned long flags; + se_sess->se_node_acl = NULL; + + /* + * Also determine if we need to drop the extra ->cmd_kref if + * it had been previously dynamically generated, and + * the endpoint is not caching dynamic ACLs. + */ + mutex_lock(&se_tpg->acl_node_mutex); + if (se_nacl->dynamic_node_acl && + !se_tfo->tpg_check_demo_mode_cache(se_tpg)) { + spin_lock_irqsave(&se_nacl->nacl_sess_lock, flags); + if (list_empty(&se_nacl->acl_sess_list)) + se_nacl->dynamic_stop = true; + spin_unlock_irqrestore(&se_nacl->nacl_sess_lock, flags); + + if (se_nacl->dynamic_stop) + list_del(&se_nacl->acl_list); + } + mutex_unlock(&se_tpg->acl_node_mutex); + + if (se_nacl->dynamic_stop) + target_put_nacl(se_nacl); + target_put_nacl(se_nacl); } if (se_sess->sess_cmd_map) { @@ -518,16 +557,12 @@ EXPORT_SYMBOL(transport_free_session); void transport_deregister_session(struct se_session *se_sess) { struct se_portal_group *se_tpg = se_sess->se_tpg; - const struct target_core_fabric_ops *se_tfo; - struct se_node_acl *se_nacl; unsigned long flags; - bool drop_nacl = false; if (!se_tpg) { transport_free_session(se_sess); return; } - se_tfo = se_tpg->se_tpg_tfo; spin_lock_irqsave(&se_tpg->session_lock, flags); list_del(&se_sess->sess_list); @@ -535,33 +570,15 @@ void transport_deregister_session(struct se_session *se_sess) se_sess->fabric_sess_ptr = NULL; spin_unlock_irqrestore(&se_tpg->session_lock, flags); - /* - * Determine if we need to do extra work for this initiator node's - * struct se_node_acl if it had been previously dynamically generated. - */ - se_nacl = se_sess->se_node_acl; - - mutex_lock(&se_tpg->acl_node_mutex); - if (se_nacl && se_nacl->dynamic_node_acl) { - if (!se_tfo->tpg_check_demo_mode_cache(se_tpg)) { - list_del(&se_nacl->acl_list); - drop_nacl = true; - } - } - mutex_unlock(&se_tpg->acl_node_mutex); - - if (drop_nacl) { - core_tpg_wait_for_nacl_pr_ref(se_nacl); - core_free_device_list_for_node(se_nacl, se_tpg); - se_sess->se_node_acl = NULL; - kfree(se_nacl); - } pr_debug("TARGET_CORE[%s]: Deregistered fabric_sess\n", se_tpg->se_tpg_tfo->get_fabric_name()); /* * If last kref is dropping now for an explicit NodeACL, awake sleeping * ->acl_free_comp caller to wakeup configfs se_node_acl->acl_group * removal context from within transport_free_session() code. + * + * For dynamic ACL, target_put_nacl() uses target_complete_nacl() + * to release all remaining generate_node_acl=1 created ACL resources. */ transport_free_session(se_sess); diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 43edf82e54ff..da854fb4530f 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -538,6 +538,7 @@ struct se_node_acl { char initiatorname[TRANSPORT_IQN_LEN]; /* Used to signal demo mode created ACL, disabled by default */ bool dynamic_node_acl; + bool dynamic_stop; u32 queue_depth; u32 acl_index; enum target_prot_type saved_prot_type; -- cgit v1.2.3 From 9b2792c3da1e80f2d460167d319302a24c9ca2b7 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Mon, 6 Feb 2017 14:28:09 -0800 Subject: target: Fix COMPARE_AND_WRITE ref leak for non GOOD status This patch addresses a long standing bug where the commit phase of COMPARE_AND_WRITE would result in a se_cmd->cmd_kref reference leak if se_cmd->scsi_status returned non SAM_STAT_GOOD. This would manifest first as a lost SCSI response, and eventual hung task during fabric driver logout or re-login, as existing shutdown logic waited for the COMPARE_AND_WRITE se_cmd->cmd_kref to reach zero. To address this bug, compare_and_write_post() has been changed to drop the incorrect !cmd->scsi_status conditional that was preventing *post_ret = 1 for being set during non SAM_STAT_GOOD status. This patch has been tested with SAM_STAT_CHECK_CONDITION status from normal target_complete_cmd() callback path, as well as the incoming __target_execute_cmd() submission failure path when se_cmd->execute_cmd() returns non zero status. Reported-by: Donald White Cc: Donald White Tested-by: Gary Guo Cc: Gary Guo Reviewed-by: Christoph Hellwig Cc: # v3.12+ Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_sbc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 4879e70e2eef..df7b6e95c019 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -451,6 +451,7 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success, int *post_ret) { struct se_device *dev = cmd->se_dev; + sense_reason_t ret = TCM_NO_SENSE; /* * Only set SCF_COMPARE_AND_WRITE_POST to force a response fall-through @@ -458,9 +459,12 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success, * sent to the backend driver. */ spin_lock_irq(&cmd->t_state_lock); - if ((cmd->transport_state & CMD_T_SENT) && !cmd->scsi_status) { + if (cmd->transport_state & CMD_T_SENT) { cmd->se_cmd_flags |= SCF_COMPARE_AND_WRITE_POST; *post_ret = 1; + + if (cmd->scsi_status == SAM_STAT_CHECK_CONDITION) + ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; } spin_unlock_irq(&cmd->t_state_lock); @@ -470,7 +474,7 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success, */ up(&dev->caw_sem); - return TCM_NO_SENSE; + return ret; } static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool success, -- cgit v1.2.3 From b22bc27868e8c11fe3f00937a341b44f80b50364 Mon Sep 17 00:00:00 2001 From: "Bryant G. Ly" Date: Mon, 6 Feb 2017 10:04:28 -0600 Subject: ibmvscsis: Add SGL limit This patch adds internal LIO sgl limit since the driver already sets a max transfer limit on transport layer of 1MB to the client. Cc: stable@vger.kernel.org Tested-by: Steven Royer Signed-off-by: Bryant G. Ly Signed-off-by: Nicholas Bellinger --- drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c index 99b747cedbeb..0f807798c624 100644 --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c @@ -3816,6 +3816,7 @@ static struct configfs_attribute *ibmvscsis_tpg_attrs[] = { static const struct target_core_fabric_ops ibmvscsis_ops = { .module = THIS_MODULE, .name = "ibmvscsis", + .max_data_sg_nents = MAX_TXU / PAGE_SIZE, .get_fabric_name = ibmvscsis_get_fabric_name, .tpg_get_wwn = ibmvscsis_get_fabric_wwn, .tpg_get_tag = ibmvscsis_get_tag, -- cgit v1.2.3