target: Fix multi-session dynamic se_node_acl double free OOPs
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@daterainc.com> Tested-by: Rob Millner <rlm@daterainc.com> Cc: Rob Millner <rlm@daterainc.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
4a3c526ced
commit
6af0acc0b6
2 changed files with 44 additions and 27 deletions
|
@ -423,8 +423,20 @@ static void target_complete_nacl(struct kref *kref)
|
||||||
{
|
{
|
||||||
struct se_node_acl *nacl = container_of(kref,
|
struct se_node_acl *nacl = container_of(kref,
|
||||||
struct se_node_acl, acl_kref);
|
struct se_node_acl, acl_kref);
|
||||||
|
struct se_portal_group *se_tpg = nacl->se_tpg;
|
||||||
|
|
||||||
complete(&nacl->acl_free_comp);
|
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)
|
void target_put_nacl(struct se_node_acl *nacl)
|
||||||
|
@ -465,12 +477,39 @@ EXPORT_SYMBOL(transport_deregister_session_configfs);
|
||||||
void transport_free_session(struct se_session *se_sess)
|
void transport_free_session(struct se_session *se_sess)
|
||||||
{
|
{
|
||||||
struct se_node_acl *se_nacl = se_sess->se_node_acl;
|
struct se_node_acl *se_nacl = se_sess->se_node_acl;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Drop the se_node_acl->nacl_kref obtained from within
|
* Drop the se_node_acl->nacl_kref obtained from within
|
||||||
* core_tpg_get_initiator_node_acl().
|
* core_tpg_get_initiator_node_acl().
|
||||||
*/
|
*/
|
||||||
if (se_nacl) {
|
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;
|
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);
|
target_put_nacl(se_nacl);
|
||||||
}
|
}
|
||||||
if (se_sess->sess_cmd_map) {
|
if (se_sess->sess_cmd_map) {
|
||||||
|
@ -484,16 +523,12 @@ EXPORT_SYMBOL(transport_free_session);
|
||||||
void transport_deregister_session(struct se_session *se_sess)
|
void transport_deregister_session(struct se_session *se_sess)
|
||||||
{
|
{
|
||||||
struct se_portal_group *se_tpg = se_sess->se_tpg;
|
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;
|
unsigned long flags;
|
||||||
bool drop_nacl = false;
|
|
||||||
|
|
||||||
if (!se_tpg) {
|
if (!se_tpg) {
|
||||||
transport_free_session(se_sess);
|
transport_free_session(se_sess);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
se_tfo = se_tpg->se_tpg_tfo;
|
|
||||||
|
|
||||||
spin_lock_irqsave(&se_tpg->session_lock, flags);
|
spin_lock_irqsave(&se_tpg->session_lock, flags);
|
||||||
list_del(&se_sess->sess_list);
|
list_del(&se_sess->sess_list);
|
||||||
|
@ -501,34 +536,15 @@ void transport_deregister_session(struct se_session *se_sess)
|
||||||
se_sess->fabric_sess_ptr = NULL;
|
se_sess->fabric_sess_ptr = NULL;
|
||||||
spin_unlock_irqrestore(&se_tpg->session_lock, flags);
|
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);
|
|
||||||
se_tpg->num_node_acls--;
|
|
||||||
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",
|
pr_debug("TARGET_CORE[%s]: Deregistered fabric_sess\n",
|
||||||
se_tpg->se_tpg_tfo->get_fabric_name());
|
se_tpg->se_tpg_tfo->get_fabric_name());
|
||||||
/*
|
/*
|
||||||
* If last kref is dropping now for an explicit NodeACL, awake sleeping
|
* If last kref is dropping now for an explicit NodeACL, awake sleeping
|
||||||
* ->acl_free_comp caller to wakeup configfs se_node_acl->acl_group
|
* ->acl_free_comp caller to wakeup configfs se_node_acl->acl_group
|
||||||
* removal context from within transport_free_session() code.
|
* 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);
|
transport_free_session(se_sess);
|
||||||
|
|
|
@ -544,6 +544,7 @@ struct se_node_acl {
|
||||||
/* Used to signal demo mode created ACL, disabled by default */
|
/* Used to signal demo mode created ACL, disabled by default */
|
||||||
bool dynamic_node_acl;
|
bool dynamic_node_acl;
|
||||||
bool acl_stop:1;
|
bool acl_stop:1;
|
||||||
|
bool dynamic_stop;
|
||||||
u32 queue_depth;
|
u32 queue_depth;
|
||||||
u32 acl_index;
|
u32 acl_index;
|
||||||
enum target_prot_type saved_prot_type;
|
enum target_prot_type saved_prot_type;
|
||||||
|
|
Loading…
Add table
Reference in a new issue