From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
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 <rlm@xxxxxxxxxxxxx>
Tested-by: Rob Millner <rlm@xxxxxxxxxxxxx>
Cc: Rob Millner <rlm@xxxxxxxxxxxxx>
Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
---
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 3bf46bf..64cd004 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;
--
1.8.5.3