Skip to contentSkip to conversation

Patch "target: Obtain se_node_acl->acl_kref during get_initiator_node_acl" has been added to the 4.4-stable tree

PATCH [email protected] 1 participant 1 message Mar 9, 2017

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

    target: Obtain se_node_acl->acl_kref during get_initiator_node_acl

to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
target-obtain-se_node_acl-acl_kref-during-get_initiator_node_acl.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 21aaa23b0ebbd19334fa461370c03cbb076b3295 Mon Sep 17 00:00:00 2001

From: Nicholas Bellinger <[email protected]>
Date: Thu, 7 Jan 2016 22:09:27 -0800
Subject: target: Obtain se_node_acl->acl_kref during get_initiator_node_acl

From: Nicholas Bellinger <[email protected]>

commit 21aaa23b0ebbd19334fa461370c03cbb076b3295 upstream.

This patch addresses a long standing race where obtaining se_node_acl->acl_kref in __transport_register_session() happens a bit too late, and leaves open the potential for core_tpg_del_initiator_node_acl() to hit a NULL pointer dereference.

Instead, take ->acl_kref in core_tpg_get_initiator_node_acl() while se_portal_group->acl_node_mutex is held, and move the final target_put_nacl() from transport_deregister_session() into transport_free_session() so that fabric driver login failure handling using the modern method to still work as expected.

Also, update core_tpg_get_initiator_node_acl() to take an extra reference for dynamically generated acls for demo-mode, before returning to fabric caller. Also update iscsi-target sendtargets special case handling to use target_tpg_has_node_acl() when checking if demo_mode_discovery == true during discovery lookup.

Note the existing wait_for_completion(&acl->acl_free_comp) in core_tpg_del_initiator_node_acl() does not change.

Cc Sagi Grimberg <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Hannes Reinecke <[email protected]>
Cc Andy Grover <[email protected]>
Cc Mike Christie <[email protected]>
---
 drivers/target/iscsi/iscsi_target.c    |    2 -
 drivers/target/target_core_tpg.c       |   42 ++++++++++++++++++++++++++++++++-
 drivers/target/target_core_transport.c |   19 ++++++++++----
 include/target/target_core_fabric.h    |    2 +
 4 files changed, 57 insertions(+), 8 deletions(-)
drivers/target/iscsi/iscsi_target.c +1 -1
@@ -3436,7 +3436,7 @@ iscsit_build_sendtargets_response(struct
 
 			if ((tpg->tpg_attrib.generate_node_acls == 0) &&
 			    (tpg->tpg_attrib.demo_mode_discovery == 0) &&
-			    (!core_tpg_get_initiator_node_acl(&tpg->tpg_se_tpg,
+			    (!target_tpg_has_node_acl(&tpg->tpg_se_tpg,
 				cmd->conn->sess->sess_ops->InitiatorName))) {
 				continue;
 			}
drivers/target/target_core_tpg.c +41 -1

62 lines not on this page · raw message

drivers/target/target_core_transport.c +13 -6
@@ -341,7 +341,6 @@ void __transport_register_session(
 					&buf[0], PR_REG_ISID_LEN);
 			se_sess->sess_bin_isid = get_unaligned_be64(&buf[0]);
 		}
-		kref_get(&se_nacl->acl_kref);
 
 		spin_lock_irq(&se_nacl->nacl_sess_lock);
 		/*
@@ -432,6 +431,7 @@ void target_put_nacl(struct se_node_acl
 {
 	kref_put(&nacl->acl_kref, target_complete_nacl);
 }
+EXPORT_SYMBOL(target_put_nacl);
 
 void transport_deregister_session_configfs(struct se_session *se_sess)
 {
@@ -464,6 +464,15 @@ EXPORT_SYMBOL(transport_deregister_sessi
 
 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) {
+		se_sess->se_node_acl = NULL;
+		target_put_nacl(se_nacl);
+	}
 	if (se_sess->sess_cmd_map) {
 		percpu_ida_destroy(&se_sess->sess_tag_pool);
 		kvfree(se_sess->sess_cmd_map);
@@ -478,7 +487,7 @@ void transport_deregister_session(struct
 	const struct target_core_fabric_ops *se_tfo;
 	struct se_node_acl *se_nacl;
 	unsigned long flags;
-	bool comp_nacl = true, drop_nacl = false;
+	bool drop_nacl = false;
 
 	if (!se_tpg) {
 		transport_free_session(se_sess);
@@ -511,18 +520,16 @@ void transport_deregister_session(struct
 	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);
-		comp_nacl = false;
 	}
 	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.
+	 * removal context from within transport_free_session() code.
 	 */
-	if (se_nacl && comp_nacl)
-		target_put_nacl(se_nacl);
 
 	transport_free_session(se_sess);
 }
include/target/target_core_fabric.h +2 -0
@@ -168,6 +168,8 @@ void	core_allocate_nexus_loss_ua(struct
 
 struct se_node_acl *core_tpg_get_initiator_node_acl(struct se_portal_group *tpg,
 		unsigned char *);
+bool	target_tpg_has_node_acl(struct se_portal_group *tpg,
+		const char *);
 struct se_node_acl *core_tpg_check_initiator_node_acl(struct se_portal_group *,
 		unsigned char *);
 int	core_tpg_set_initiator_node_queue_depth(struct se_portal_group *,

Patches currently in stable-queue which might be from [email protected] are

queue-4.4/target-fix-multi-session-dynamic-se_node_acl-double-free-oops.patch queue-4.4/target-obtain-se_node_acl-acl_kref-during-get_initiator_node_acl.patch