From 05c5dd75d77c8176c2beef56471868e29c19b47c Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Thu, 23 Mar 2017 17:19:24 -0700 Subject: iscsi-target: Fix TMR reference leak during session shutdown commit efb2ea770bb3b0f40007530bc8b0c22f36e1c5eb upstream. This patch fixes a iscsi-target specific TMR reference leak during session shutdown, that could occur when a TMR was quiesced before the hand-off back to iscsi-target code via transport_cmd_check_stop_to_fabric(). The reference leak happens because iscsit_free_cmd() was incorrectly skipping the final target_put_sess_cmd() for TMRs when transport_generic_free_cmd() returned zero because the se_cmd->cmd_kref did not reach zero, due to the missing se_cmd assignment in original code. The result was iscsi_cmd and it's associated se_cmd memory would be freed once se_sess->sess_cmd_map where released, but the associated se_tmr_req was leaked and remained part of se_device->dev_tmr_list. This bug would manfiest itself as kernel paging request OOPsen in core_tmr_lun_reset(), when a left-over se_tmr_req attempted to dereference it's se_cmd pointer that had already been released during normal session shutdown. To address this bug, go ahead and treat ISCSI_OP_SCSI_CMD and ISCSI_OP_SCSI_TMFUNC the same when there is an extra se_cmd->cmd_kref to drop in iscsit_free_cmd(), and use op_scsi to signal __iscsit_free_cmd() when the former needs to clear any further iscsi related I/O state. Reported-by: Rob Millner Cc: Rob Millner Reported-by: Chu Yuan Lin Cc: Chu Yuan Lin Tested-by: Chu Yuan Lin Signed-off-by: Nicholas Bellinger Signed-off-by: Greg Kroah-Hartman --- drivers/target/iscsi/iscsi_target_util.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 428b0d9e3dba..93590521ae33 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -731,21 +731,23 @@ void iscsit_free_cmd(struct iscsi_cmd *cmd, bool shutdown) { struct se_cmd *se_cmd = NULL; int rc; + bool op_scsi = false; /* * Determine if a struct se_cmd is associated with * this struct iscsi_cmd. */ switch (cmd->iscsi_opcode) { case ISCSI_OP_SCSI_CMD: - se_cmd = &cmd->se_cmd; - __iscsit_free_cmd(cmd, true, shutdown); + op_scsi = true; /* * Fallthrough */ case ISCSI_OP_SCSI_TMFUNC: - rc = transport_generic_free_cmd(&cmd->se_cmd, shutdown); - if (!rc && shutdown && se_cmd && se_cmd->se_sess) { - __iscsit_free_cmd(cmd, true, shutdown); + se_cmd = &cmd->se_cmd; + __iscsit_free_cmd(cmd, op_scsi, shutdown); + rc = transport_generic_free_cmd(se_cmd, shutdown); + if (!rc && shutdown && se_cmd->se_sess) { + __iscsit_free_cmd(cmd, op_scsi, shutdown); target_put_sess_cmd(se_cmd); } break; -- cgit v1.2.3 From 1e1de2e841e141991250d7c0ac79c5877a43b6a4 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Sun, 2 Apr 2017 13:36:44 -0700 Subject: iscsi-target: Drop work-around for legacy GlobalSAN initiator commit 1c99de981f30b3e7868b8d20ce5479fa1c0fea46 upstream. Once upon a time back in 2009, a work-around was added to support the GlobalSAN iSCSI initiator v3.3 for MacOSX, which during login did not propose nor respond to MaxBurstLength, FirstBurstLength, DefaultTime2Wait and DefaultTime2Retain keys. The work-around in iscsi_check_proposer_for_optional_reply() allowed the missing keys to be proposed, but did not require waiting for a response before moving to full feature phase operation. This allowed GlobalSAN v3.3 to work out-of-the box, and for many years we didn't run into login interopt issues with any other initiators.. Until recently, when Martin tried a QLogic 57840S iSCSI Offload HBA on Windows 2016 which completed login, but subsequently failed with: Got unknown iSCSI OpCode: 0x43 The issue was QLogic MSFT side did not propose DefaultTime2Wait + DefaultTime2Retain, so LIO proposes them itself, and immediately transitions to full feature phase because of the GlobalSAN hack. However, the QLogic MSFT side still attempts to respond to DefaultTime2Retain + DefaultTime2Wait, even though LIO has set ISCSI_FLAG_LOGIN_NEXT_STAGE3 + ISCSI_FLAG_LOGIN_TRANSIT in last login response. So while the QLogic MSFT side should have been proposing these two keys to start, it was doing the correct thing per RFC-3720 attempting to respond to proposed keys before transitioning to full feature phase. All that said, recent versions of GlobalSAN iSCSI (v5.3.0.541) does correctly propose the four keys during login, making the original work-around moot. So in order to allow QLogic MSFT to run unmodified as-is, go ahead and drop this long standing work-around. Reported-by: Martin Svec Cc: Martin Svec Cc: Himanshu Madhani Cc: Arun Easi Signed-off-by: Nicholas Bellinger Signed-off-by: Greg Kroah-Hartman --- drivers/target/iscsi/iscsi_target_parameters.c | 16 ---------------- 1 file changed, 16 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c index 2cbea2af7cd0..6d1b0acbc5b3 100644 --- a/drivers/target/iscsi/iscsi_target_parameters.c +++ b/drivers/target/iscsi/iscsi_target_parameters.c @@ -780,22 +780,6 @@ static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param) } else if (IS_TYPE_NUMBER(param)) { if (!strcmp(param->name, MAXRECVDATASEGMENTLENGTH)) SET_PSTATE_REPLY_OPTIONAL(param); - /* - * The GlobalSAN iSCSI Initiator for MacOSX does - * not respond to MaxBurstLength, FirstBurstLength, - * DefaultTime2Wait or DefaultTime2Retain parameter keys. - * So, we set them to 'reply optional' here, and assume the - * the defaults from iscsi_parameters.h if the initiator - * is not RFC compliant and the keys are not negotiated. - */ - if (!strcmp(param->name, MAXBURSTLENGTH)) - SET_PSTATE_REPLY_OPTIONAL(param); - if (!strcmp(param->name, FIRSTBURSTLENGTH)) - SET_PSTATE_REPLY_OPTIONAL(param); - if (!strcmp(param->name, DEFAULTTIME2WAIT)) - SET_PSTATE_REPLY_OPTIONAL(param); - if (!strcmp(param->name, DEFAULTTIME2RETAIN)) - SET_PSTATE_REPLY_OPTIONAL(param); /* * Required for gPXE iSCSI boot client */ -- cgit v1.2.3 From a1c2b01c310afa6490db8a7f704b8fe77f7107fd Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Tue, 11 Apr 2017 16:24:16 -0700 Subject: target: Fix compare_and_write_callback handling for non GOOD status commit a71a5dc7f833943998e97ca8fa6a4c708a0ed1a9 upstream. Following the bugfix for handling non SAM_STAT_GOOD COMPARE_AND_WRITE status during COMMIT phase in commit 9b2792c3da1, the same bug exists for the READ phase as well. 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_callback() has been changed to set post_ret = 1 and return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE as necessary to signal failure status. Reported-by: Bill Borsari Cc: Bill Borsari Tested-by: Gary Guo Cc: Gary Guo Signed-off-by: Nicholas Bellinger Signed-off-by: Greg Kroah-Hartman --- drivers/target/target_core_sbc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 90c5dffc9fa4..608117819366 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -498,8 +498,11 @@ static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes * been failed with a non-zero SCSI status. */ if (cmd->scsi_status) { - pr_err("compare_and_write_callback: non zero scsi_status:" + pr_debug("compare_and_write_callback: non zero scsi_status:" " 0x%02x\n", cmd->scsi_status); + *post_ret = 1; + if (cmd->scsi_status == SAM_STAT_CHECK_CONDITION) + ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; goto out; } -- cgit v1.2.3 From de41b0e12d6edf310f77c7bacc7a05458a0a95d1 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 4 May 2017 15:50:47 -0700 Subject: target/fileio: Fix zero-length READ and WRITE handling commit 59ac9c078141b8fd0186c0b18660a1b2c24e724e upstream. This patch fixes zero-length READ and WRITE handling in target/FILEIO, which was broken a long time back by: Since: commit d81cb44726f050d7cf1be4afd9cb45d153b52066 Author: Paolo Bonzini Date: Mon Sep 17 16:36:11 2012 -0700 target: go through normal processing for all zero-length commands which moved zero-length READ and WRITE completion out of target-core, to doing submission into backend driver code. To address this, go ahead and invoke target_complete_cmd() for any non negative return value in fd_do_rw(). Signed-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig Cc: Andy Grover Cc: David Disseldorp Signed-off-by: Nicholas Bellinger Signed-off-by: Greg Kroah-Hartman --- drivers/target/target_core_file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index 79291869bce6..041a56987845 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -594,8 +594,7 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, if (ret < 0) return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; - if (ret) - target_complete_cmd(cmd, SAM_STAT_GOOD); + target_complete_cmd(cmd, SAM_STAT_GOOD); return 0; } -- cgit v1.2.3 From 6cd0200a95545d63187070afc03b37c61ef17a04 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Thu, 7 Jan 2016 22:15:06 -0800 Subject: target: Convert ACL change queue_depth se_session reference usage commit d36ad77f702356afb1009d2987b0ab55da4c7d57 upstream. This patch converts core_tpg_set_initiator_node_queue_depth() to use struct se_node_acl->acl_sess_list when performing explicit se_tpg_tfo->shutdown_session() for active sessions, in order for new se_node_acl->queue_depth to take effect. This follows how core_tpg_del_initiator_node_acl() currently works when invoking se_tpg_tfo->shutdown-session(), and ahead of the next patch to take se_node_acl->acl_kref during lookup, the extra get_initiator_node_acl() can go away. In order to achieve this, go ahead and change target_get_session() to use kref_get_unless_zero() and propigate up the return value to know when a session is already being released. This is because se_node_acl->acl_group is already protecting se_node_acl->acl_group reference via configfs, and shutdown within core_tpg_del_initiator_node_acl() won't occur until sys_write() to core_tpg_set_initiator_node_queue_depth() attribute returns back to user-space. Also, drop the left-over iscsi-target hack, and obtain se_portal_group->session_lock in lio_tpg_shutdown_session() internally. Remove iscsi-target wrapper and unused se_tpg + force parameters and associated code. Reported-by: Christoph Hellwig Cc: Sagi Grimberg Cc: Hannes Reinecke Cc: Andy Grover Cc: Mike Christie Signed-off-by: Nicholas Bellinger Signed-off-by: Greg Kroah-Hartman --- drivers/target/iscsi/iscsi_target_configfs.c | 29 ++--- drivers/target/iscsi/iscsi_target_tpg.c | 10 -- drivers/target/iscsi/iscsi_target_tpg.h | 2 - drivers/target/target_core_tpg.c | 152 ++++++++------------------- drivers/target/target_core_transport.c | 4 +- 5 files changed, 54 insertions(+), 143 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c index b4bfd706ac94..2f821de63049 100644 --- a/drivers/target/iscsi/iscsi_target_configfs.c +++ b/drivers/target/iscsi/iscsi_target_configfs.c @@ -725,11 +725,8 @@ static ssize_t lio_target_nacl_cmdsn_depth_store(struct config_item *item, if (iscsit_get_tpg(tpg) < 0) return -EINVAL; - /* - * iscsit_tpg_set_initiator_node_queue_depth() assumes force=1 - */ - ret = iscsit_tpg_set_initiator_node_queue_depth(tpg, - config_item_name(acl_ci), cmdsn_depth, 1); + + ret = core_tpg_set_initiator_node_queue_depth(se_nacl, cmdsn_depth); pr_debug("LIO_Target_ConfigFS: %s/%s Set CmdSN Window: %u for" "InitiatorName: %s\n", config_item_name(wwn_ci), @@ -1593,42 +1590,30 @@ static int lio_tpg_check_prot_fabric_only( } /* - * Called with spin_lock_irq(struct se_portal_group->session_lock) held - * or not held. - * - * Also, this function calls iscsit_inc_session_usage_count() on the + * This function calls iscsit_inc_session_usage_count() on the * struct iscsi_session in question. */ static int lio_tpg_shutdown_session(struct se_session *se_sess) { struct iscsi_session *sess = se_sess->fabric_sess_ptr; - struct se_portal_group *se_tpg = se_sess->se_tpg; - bool local_lock = false; - - if (!spin_is_locked(&se_tpg->session_lock)) { - spin_lock_irq(&se_tpg->session_lock); - local_lock = true; - } + struct se_portal_group *se_tpg = &sess->tpg->tpg_se_tpg; + spin_lock_bh(&se_tpg->session_lock); spin_lock(&sess->conn_lock); if (atomic_read(&sess->session_fall_back_to_erl0) || atomic_read(&sess->session_logout) || (sess->time2retain_timer_flags & ISCSI_TF_EXPIRED)) { spin_unlock(&sess->conn_lock); - if (local_lock) - spin_unlock_irq(&sess->conn_lock); + spin_unlock_bh(&se_tpg->session_lock); return 0; } atomic_set(&sess->session_reinstatement, 1); spin_unlock(&sess->conn_lock); iscsit_stop_time2retain_timer(sess); - spin_unlock_irq(&se_tpg->session_lock); + spin_unlock_bh(&se_tpg->session_lock); iscsit_stop_session(sess, 1, 1); - if (!local_lock) - spin_lock_irq(&se_tpg->session_lock); - return 1; } diff --git a/drivers/target/iscsi/iscsi_target_tpg.c b/drivers/target/iscsi/iscsi_target_tpg.c index 68261b7dcefe..205a509b0dfb 100644 --- a/drivers/target/iscsi/iscsi_target_tpg.c +++ b/drivers/target/iscsi/iscsi_target_tpg.c @@ -589,16 +589,6 @@ int iscsit_tpg_del_network_portal( return iscsit_tpg_release_np(tpg_np, tpg, np); } -int iscsit_tpg_set_initiator_node_queue_depth( - struct iscsi_portal_group *tpg, - unsigned char *initiatorname, - u32 queue_depth, - int force) -{ - return core_tpg_set_initiator_node_queue_depth(&tpg->tpg_se_tpg, - initiatorname, queue_depth, force); -} - int iscsit_ta_authentication(struct iscsi_portal_group *tpg, u32 authentication) { unsigned char buf1[256], buf2[256], *none = NULL; diff --git a/drivers/target/iscsi/iscsi_target_tpg.h b/drivers/target/iscsi/iscsi_target_tpg.h index 9db32bd24cd4..2da211920c18 100644 --- a/drivers/target/iscsi/iscsi_target_tpg.h +++ b/drivers/target/iscsi/iscsi_target_tpg.h @@ -26,8 +26,6 @@ extern struct iscsi_tpg_np *iscsit_tpg_add_network_portal(struct iscsi_portal_gr int); extern int iscsit_tpg_del_network_portal(struct iscsi_portal_group *, struct iscsi_tpg_np *); -extern int iscsit_tpg_set_initiator_node_queue_depth(struct iscsi_portal_group *, - unsigned char *, u32, int); extern int iscsit_ta_authentication(struct iscsi_portal_group *, u32); extern int iscsit_ta_login_timeout(struct iscsi_portal_group *, u32); extern int iscsit_ta_netif_timeout(struct iscsi_portal_group *, u32); diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c index 2794c6ec5c3c..899c33b3c734 100644 --- a/drivers/target/target_core_tpg.c +++ b/drivers/target/target_core_tpg.c @@ -169,28 +169,25 @@ void core_tpg_add_node_to_devs( mutex_unlock(&tpg->tpg_lun_mutex); } -/* core_set_queue_depth_for_node(): - * - * - */ -static int core_set_queue_depth_for_node( - struct se_portal_group *tpg, - struct se_node_acl *acl) +static void +target_set_nacl_queue_depth(struct se_portal_group *tpg, + struct se_node_acl *acl, u32 queue_depth) { + acl->queue_depth = queue_depth; + if (!acl->queue_depth) { - pr_err("Queue depth for %s Initiator Node: %s is 0," + pr_warn("Queue depth for %s Initiator Node: %s is 0," "defaulting to 1.\n", tpg->se_tpg_tfo->get_fabric_name(), acl->initiatorname); acl->queue_depth = 1; } - - return 0; } static struct se_node_acl *target_alloc_node_acl(struct se_portal_group *tpg, const unsigned char *initiatorname) { struct se_node_acl *acl; + u32 queue_depth; acl = kzalloc(max(sizeof(*acl), tpg->se_tpg_tfo->node_acl_size), GFP_KERNEL); @@ -205,24 +202,20 @@ static struct se_node_acl *target_alloc_node_acl(struct se_portal_group *tpg, spin_lock_init(&acl->nacl_sess_lock); mutex_init(&acl->lun_entry_mutex); atomic_set(&acl->acl_pr_ref_count, 0); + if (tpg->se_tpg_tfo->tpg_get_default_depth) - acl->queue_depth = tpg->se_tpg_tfo->tpg_get_default_depth(tpg); + queue_depth = tpg->se_tpg_tfo->tpg_get_default_depth(tpg); else - acl->queue_depth = 1; + queue_depth = 1; + target_set_nacl_queue_depth(tpg, acl, queue_depth); + snprintf(acl->initiatorname, TRANSPORT_IQN_LEN, "%s", initiatorname); acl->se_tpg = tpg; acl->acl_index = scsi_get_new_index(SCSI_AUTH_INTR_INDEX); tpg->se_tpg_tfo->set_default_node_attributes(acl); - if (core_set_queue_depth_for_node(tpg, acl) < 0) - goto out_free_acl; - return acl; - -out_free_acl: - kfree(acl); - return NULL; } static void target_add_node_acl(struct se_node_acl *acl) @@ -369,7 +362,8 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl *acl) if (sess->sess_tearing_down != 0) continue; - target_get_session(sess); + if (!target_get_session(sess)) + continue; list_move(&sess->sess_acl_list, &sess_list); } spin_unlock_irqrestore(&acl->nacl_sess_lock, flags); @@ -406,108 +400,52 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl *acl) * */ int core_tpg_set_initiator_node_queue_depth( - struct se_portal_group *tpg, - unsigned char *initiatorname, - u32 queue_depth, - int force) + struct se_node_acl *acl, + u32 queue_depth) { - struct se_session *sess, *init_sess = NULL; - struct se_node_acl *acl; + LIST_HEAD(sess_list); + struct se_portal_group *tpg = acl->se_tpg; + struct se_session *sess, *sess_tmp; unsigned long flags; - int dynamic_acl = 0; - - mutex_lock(&tpg->acl_node_mutex); - acl = __core_tpg_get_initiator_node_acl(tpg, initiatorname); - if (!acl) { - pr_err("Access Control List entry for %s Initiator" - " Node %s does not exists for TPG %hu, ignoring" - " request.\n", tpg->se_tpg_tfo->get_fabric_name(), - initiatorname, tpg->se_tpg_tfo->tpg_get_tag(tpg)); - mutex_unlock(&tpg->acl_node_mutex); - return -ENODEV; - } - if (acl->dynamic_node_acl) { - acl->dynamic_node_acl = 0; - dynamic_acl = 1; - } - mutex_unlock(&tpg->acl_node_mutex); - - spin_lock_irqsave(&tpg->session_lock, flags); - list_for_each_entry(sess, &tpg->tpg_sess_list, sess_list) { - if (sess->se_node_acl != acl) - continue; - - if (!force) { - pr_err("Unable to change queue depth for %s" - " Initiator Node: %s while session is" - " operational. To forcefully change the queue" - " depth and force session reinstatement" - " use the \"force=1\" parameter.\n", - tpg->se_tpg_tfo->get_fabric_name(), initiatorname); - spin_unlock_irqrestore(&tpg->session_lock, flags); - - mutex_lock(&tpg->acl_node_mutex); - if (dynamic_acl) - acl->dynamic_node_acl = 1; - mutex_unlock(&tpg->acl_node_mutex); - return -EEXIST; - } - /* - * Determine if the session needs to be closed by our context. - */ - if (!tpg->se_tpg_tfo->shutdown_session(sess)) - continue; - - init_sess = sess; - break; - } + int rc; /* * User has requested to change the queue depth for a Initiator Node. * Change the value in the Node's struct se_node_acl, and call - * core_set_queue_depth_for_node() to add the requested queue depth. - * - * Finally call tpg->se_tpg_tfo->close_session() to force session - * reinstatement to occur if there is an active session for the - * $FABRIC_MOD Initiator Node in question. + * target_set_nacl_queue_depth() to set the new queue depth. */ - acl->queue_depth = queue_depth; + target_set_nacl_queue_depth(tpg, acl, queue_depth); + + spin_lock_irqsave(&acl->nacl_sess_lock, flags); + list_for_each_entry_safe(sess, sess_tmp, &acl->acl_sess_list, + sess_acl_list) { + if (sess->sess_tearing_down != 0) + continue; + if (!target_get_session(sess)) + continue; + spin_unlock_irqrestore(&acl->nacl_sess_lock, flags); - if (core_set_queue_depth_for_node(tpg, acl) < 0) { - spin_unlock_irqrestore(&tpg->session_lock, flags); /* - * Force session reinstatement if - * core_set_queue_depth_for_node() failed, because we assume - * the $FABRIC_MOD has already the set session reinstatement - * bit from tpg->se_tpg_tfo->shutdown_session() called above. + * Finally call tpg->se_tpg_tfo->close_session() to force session + * reinstatement to occur if there is an active session for the + * $FABRIC_MOD Initiator Node in question. */ - if (init_sess) - tpg->se_tpg_tfo->close_session(init_sess); - - mutex_lock(&tpg->acl_node_mutex); - if (dynamic_acl) - acl->dynamic_node_acl = 1; - mutex_unlock(&tpg->acl_node_mutex); - return -EINVAL; + rc = tpg->se_tpg_tfo->shutdown_session(sess); + target_put_session(sess); + if (!rc) { + spin_lock_irqsave(&acl->nacl_sess_lock, flags); + continue; + } + target_put_session(sess); + spin_lock_irqsave(&acl->nacl_sess_lock, flags); } - spin_unlock_irqrestore(&tpg->session_lock, flags); - /* - * If the $FABRIC_MOD session for the Initiator Node ACL exists, - * forcefully shutdown the $FABRIC_MOD session/nexus. - */ - if (init_sess) - tpg->se_tpg_tfo->close_session(init_sess); + spin_unlock_irqrestore(&acl->nacl_sess_lock, flags); pr_debug("Successfully changed queue depth to: %d for Initiator" - " Node: %s on %s Target Portal Group: %u\n", queue_depth, - initiatorname, tpg->se_tpg_tfo->get_fabric_name(), + " Node: %s on %s Target Portal Group: %u\n", acl->queue_depth, + acl->initiatorname, tpg->se_tpg_tfo->get_fabric_name(), tpg->se_tpg_tfo->tpg_get_tag(tpg)); - mutex_lock(&tpg->acl_node_mutex); - if (dynamic_acl) - acl->dynamic_node_acl = 1; - mutex_unlock(&tpg->acl_node_mutex); - return 0; } EXPORT_SYMBOL(core_tpg_set_initiator_node_queue_depth); diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index df2059984e14..af301414a9f3 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -383,9 +383,9 @@ static void target_release_session(struct kref *kref) se_tpg->se_tpg_tfo->close_session(se_sess); } -void target_get_session(struct se_session *se_sess) +int target_get_session(struct se_session *se_sess) { - kref_get(&se_sess->sess_kref); + return kref_get_unless_zero(&se_sess->sess_kref); } EXPORT_SYMBOL(target_get_session); -- cgit v1.2.3 From 8e1a740a5d66f97acf2b04a6ed85941a2eb4828e Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Tue, 25 Apr 2017 10:55:12 -0700 Subject: iscsi-target: Set session_fall_back_to_erl0 when forcing reinstatement commit 197b806ae5db60c6f609d74da04ddb62ea5e1b00 upstream. While testing modification of per se_node_acl queue_depth forcing session reinstatement via lio_target_nacl_cmdsn_depth_store() -> core_tpg_set_initiator_node_queue_depth(), a hung task bug triggered when changing cmdsn_depth invoked session reinstatement while an iscsi login was already waiting for session reinstatement to complete. This can happen when an outstanding se_cmd descriptor is taking a long time to complete, and session reinstatement from iscsi login or cmdsn_depth change occurs concurrently. To address this bug, explicitly set session_fall_back_to_erl0 = 1 when forcing session reinstatement, so session reinstatement is not attempted if an active session is already being shutdown. This patch has been tested with two scenarios. The first when iscsi login is blocked waiting for iscsi session reinstatement to complete followed by queue_depth change via configfs, and second when queue_depth change via configfs us blocked followed by a iscsi login driven session reinstatement. Note this patch depends on commit d36ad77f702 to handle multiple sessions per se_node_acl when changing cmdsn_depth, and for pre v4.5 kernels will need to be included for stable as well. Reported-by: Gary Guo Tested-by: Gary Guo Cc: Gary Guo Signed-off-by: Nicholas Bellinger Signed-off-by: Greg Kroah-Hartman --- drivers/target/iscsi/iscsi_target.c | 1 + drivers/target/iscsi/iscsi_target_configfs.c | 1 + drivers/target/iscsi/iscsi_target_login.c | 1 + 3 files changed, 3 insertions(+) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 6ed80b05d674..200d3de8bc1e 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -4821,6 +4821,7 @@ int iscsit_release_sessions_for_tpg(struct iscsi_portal_group *tpg, int force) continue; } atomic_set(&sess->session_reinstatement, 1); + atomic_set(&sess->session_fall_back_to_erl0, 1); spin_unlock(&sess->conn_lock); list_move_tail(&se_sess->sess_list, &free_list); diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c index 2f821de63049..dc1bd1f1bdfe 100644 --- a/drivers/target/iscsi/iscsi_target_configfs.c +++ b/drivers/target/iscsi/iscsi_target_configfs.c @@ -1608,6 +1608,7 @@ static int lio_tpg_shutdown_session(struct se_session *se_sess) return 0; } atomic_set(&sess->session_reinstatement, 1); + atomic_set(&sess->session_fall_back_to_erl0, 1); spin_unlock(&sess->conn_lock); iscsit_stop_time2retain_timer(sess); diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index 316f66172335..4a137b0ae3dc 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -195,6 +195,7 @@ int iscsi_check_for_session_reinstatement(struct iscsi_conn *conn) initiatorname_param->value) && (sess_p->sess_ops->SessionType == sessiontype))) { atomic_set(&sess_p->session_reinstatement, 1); + atomic_set(&sess_p->session_fall_back_to_erl0, 1); spin_unlock(&sess_p->conn_lock); iscsit_inc_session_usage_count(sess_p); iscsit_stop_time2retain_timer(sess_p); -- cgit v1.2.3 From 934d0a9f9c65d31cd76c376948128afe3da526a1 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Thu, 11 May 2017 01:07:24 -0700 Subject: target: Re-add check to reject control WRITEs with overflow data commit 4ff83daa0200affe1894bd33d17bac404e3d78d4 upstream. During v4.3 when the overflow/underflow check was relaxed by commit c72c525022: commit c72c5250224d475614a00c1d7e54a67f77cd3410 Author: Roland Dreier Date: Wed Jul 22 15:08:18 2015 -0700 target: allow underflow/overflow for PR OUT etc. commands to allow underflow/overflow for Windows compliance + FCP, a consequence was to allow control CDBs to process overflow data for iscsi-target with immediate data as well. As per Roland's original change, continue to allow underflow cases for control CDBs to make Windows compliance + FCP happy, but until overflow for control CDBs is supported tree-wide, explicitly reject all control WRITEs with overflow following pre v4.3.y logic. Reported-by: Bart Van Assche Cc: Roland Dreier Signed-off-by: Nicholas Bellinger Signed-off-by: Greg Kroah-Hartman --- drivers/target/target_core_transport.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index af301414a9f3..60743bf27f37 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1154,15 +1154,28 @@ target_cmd_size_check(struct se_cmd *cmd, unsigned int size) if (cmd->unknown_data_length) { cmd->data_length = size; } else if (size != cmd->data_length) { - pr_warn("TARGET_CORE[%s]: Expected Transfer Length:" + pr_warn_ratelimited("TARGET_CORE[%s]: Expected Transfer Length:" " %u does not match SCSI CDB Length: %u for SAM Opcode:" " 0x%02x\n", cmd->se_tfo->get_fabric_name(), cmd->data_length, size, cmd->t_task_cdb[0]); - if (cmd->data_direction == DMA_TO_DEVICE && - cmd->se_cmd_flags & SCF_SCSI_DATA_CDB) { - pr_err("Rejecting underflow/overflow WRITE data\n"); - return TCM_INVALID_CDB_FIELD; + if (cmd->data_direction == DMA_TO_DEVICE) { + if (cmd->se_cmd_flags & SCF_SCSI_DATA_CDB) { + pr_err_ratelimited("Rejecting underflow/overflow" + " for WRITE data CDB\n"); + return TCM_INVALID_CDB_FIELD; + } + /* + * Some fabric drivers like iscsi-target still expect to + * always reject overflow writes. Reject this case until + * full fabric driver level support for overflow writes + * is introduced tree-wide. + */ + if (size > cmd->data_length) { + pr_err_ratelimited("Rejecting overflow for" + " WRITE control CDB\n"); + return TCM_INVALID_CDB_FIELD; + } } /* * Reject READ_* or WRITE_* with overflow/underflow for -- cgit v1.2.3