Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 01d4d673 authored by Nicholas Bellinger's avatar Nicholas Bellinger
Browse files

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: default avatarRob Millner <rlm@daterainc.com>
Tested-by: default avatarRob Millner <rlm@daterainc.com>
Cc: Rob Millner <rlm@daterainc.com>
Cc: stable@vger.kernel.org # 4.1+
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent c54eeffb

drivers/target/target_core_transport.c

+43 −26
Original line number Diff line number Diff line
@@ -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;



	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);

include/target/target_core_base.h

+1 −0
Original line number Diff line number Diff line
@@ -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;