From patchwork Mon Aug 7 03:25:30 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 9884351 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id ABC2E602CC for ; Mon, 7 Aug 2017 03:19:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A0C6D2856B for ; Mon, 7 Aug 2017 03:19:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 95A6628573; Mon, 7 Aug 2017 03:19:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D29A92856B for ; Mon, 7 Aug 2017 03:19:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751443AbdHGDTh (ORCPT ); Sun, 6 Aug 2017 23:19:37 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:38738 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751425AbdHGDTg (ORCPT ); Sun, 6 Aug 2017 23:19:36 -0400 Received: from linux-iscsi.org (localhost [127.0.0.1]) by linux-iscsi.org (Postfix) with ESMTP id 4F45540B27; Mon, 7 Aug 2017 03:25:39 +0000 (UTC) From: "Nicholas A. Bellinger" To: target-devel Cc: stable , Sasha Levin , Greg-KH , Nicholas Bellinger , Rob Millner Subject: [PATCH-v4.1.y 2/7] target: Fix multi-session dynamic se_node_acl double free OOPs Date: Mon, 7 Aug 2017 03:25:30 +0000 Message-Id: <1502076335-20271-3-git-send-email-nab@linux-iscsi.org> X-Mailer: git-send-email 1.7.2.5 In-Reply-To: <1502076335-20271-1-git-send-email-nab@linux-iscsi.org> References: <1502076335-20271-1-git-send-email-nab@linux-iscsi.org> Sender: target-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: target-devel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Nicholas Bellinger commit 01d4d673558985d9a118e1e05026633c3e2ade9b upstream. 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 Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 72 ++++++++++++++++++++++------------ include/target/target_core_base.h | 1 + 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 09cce69..5ad27a8 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -447,8 +447,23 @@ 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; + const struct target_core_fabric_ops *se_tfo = se_tpg->se_tpg_tfo; + unsigned long flags; + + if (!nacl->dynamic_stop) { + complete(&nacl->acl_free_comp); + return; + } + + spin_lock_irqsave(&se_tpg->acl_node_lock, flags); + list_del(&nacl->acl_list); + se_tpg->num_node_acls--; + spin_unlock_irqrestore(&se_tpg->acl_node_lock, flags); - complete(&nacl->acl_free_comp); + core_tpg_wait_for_nacl_pr_ref(nacl); + core_free_device_list_for_node(nacl, se_tpg); + se_tfo->tpg_release_fabric_acl(se_tpg, nacl); } void target_put_nacl(struct se_node_acl *nacl) @@ -489,12 +504,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. + */ + spin_lock_irqsave(&se_tpg->acl_node_lock, flags); + if (se_nacl->dynamic_node_acl && + !se_tfo->tpg_check_demo_mode_cache(se_tpg)) { + spin_lock(&se_nacl->nacl_sess_lock); + if (list_empty(&se_nacl->acl_sess_list)) + se_nacl->dynamic_stop = true; + spin_unlock(&se_nacl->nacl_sess_lock); + + if (se_nacl->dynamic_stop) + list_del(&se_nacl->acl_list); + } + spin_unlock_irqrestore(&se_tpg->acl_node_lock, flags); + + if (se_nacl->dynamic_stop) + target_put_nacl(se_nacl); + target_put_nacl(se_nacl); } if (se_sess->sess_cmd_map) { @@ -511,15 +553,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; 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); @@ -527,34 +566,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; - - spin_lock_irqsave(&se_tpg->acl_node_lock, flags); - if (se_nacl && se_nacl->dynamic_node_acl) { - if (!se_tfo->tpg_check_demo_mode_cache(se_tpg)) { - list_del(&se_nacl->acl_list); - se_tpg->num_node_acls--; - spin_unlock_irqrestore(&se_tpg->acl_node_lock, flags); - 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; - se_tfo->tpg_release_fabric_acl(se_tpg, se_nacl); - - spin_lock_irqsave(&se_tpg->acl_node_lock, flags); - } - } - spin_unlock_irqrestore(&se_tpg->acl_node_lock, flags); - 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 2b40a1f..3ab9ddb 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -590,6 +590,7 @@ struct se_node_acl { /* Used to signal demo mode created ACL, disabled by default */ bool dynamic_node_acl; bool acl_stop:1; + bool dynamic_stop; u32 queue_depth; u32 acl_index; enum target_prot_type saved_prot_type;