Commit 21aaa23b authored by Nicholas Bellinger's avatar Nicholas Bellinger

target: Obtain se_node_acl->acl_kref during get_initiator_node_acl

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 <sagig@mellanox.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Andy Grover <agrover@redhat.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent d36ad77f
...@@ -3435,7 +3435,7 @@ iscsit_build_sendtargets_response(struct iscsi_cmd *cmd, ...@@ -3435,7 +3435,7 @@ iscsit_build_sendtargets_response(struct iscsi_cmd *cmd,
if ((tpg->tpg_attrib.generate_node_acls == 0) && if ((tpg->tpg_attrib.generate_node_acls == 0) &&
(tpg->tpg_attrib.demo_mode_discovery == 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))) { cmd->conn->sess->sess_ops->InitiatorName))) {
continue; continue;
} }
......
...@@ -75,9 +75,21 @@ struct se_node_acl *core_tpg_get_initiator_node_acl( ...@@ -75,9 +75,21 @@ struct se_node_acl *core_tpg_get_initiator_node_acl(
unsigned char *initiatorname) unsigned char *initiatorname)
{ {
struct se_node_acl *acl; struct se_node_acl *acl;
/*
* Obtain se_node_acl->acl_kref using fabric driver provided
* initiatorname[] during node acl endpoint lookup driven by
* new se_session login.
*
* The reference is held until se_session shutdown -> release
* occurs via fabric driver invoked transport_deregister_session()
* or transport_free_session() code.
*/
mutex_lock(&tpg->acl_node_mutex); mutex_lock(&tpg->acl_node_mutex);
acl = __core_tpg_get_initiator_node_acl(tpg, initiatorname); acl = __core_tpg_get_initiator_node_acl(tpg, initiatorname);
if (acl) {
if (!kref_get_unless_zero(&acl->acl_kref))
acl = NULL;
}
mutex_unlock(&tpg->acl_node_mutex); mutex_unlock(&tpg->acl_node_mutex);
return acl; return acl;
...@@ -224,6 +236,25 @@ static void target_add_node_acl(struct se_node_acl *acl) ...@@ -224,6 +236,25 @@ static void target_add_node_acl(struct se_node_acl *acl)
acl->initiatorname); acl->initiatorname);
} }
bool target_tpg_has_node_acl(struct se_portal_group *tpg,
const char *initiatorname)
{
struct se_node_acl *acl;
bool found = false;
mutex_lock(&tpg->acl_node_mutex);
list_for_each_entry(acl, &tpg->acl_node_list, acl_list) {
if (!strcmp(acl->initiatorname, initiatorname)) {
found = true;
break;
}
}
mutex_unlock(&tpg->acl_node_mutex);
return found;
}
EXPORT_SYMBOL(target_tpg_has_node_acl);
struct se_node_acl *core_tpg_check_initiator_node_acl( struct se_node_acl *core_tpg_check_initiator_node_acl(
struct se_portal_group *tpg, struct se_portal_group *tpg,
unsigned char *initiatorname) unsigned char *initiatorname)
...@@ -240,6 +271,15 @@ struct se_node_acl *core_tpg_check_initiator_node_acl( ...@@ -240,6 +271,15 @@ struct se_node_acl *core_tpg_check_initiator_node_acl(
acl = target_alloc_node_acl(tpg, initiatorname); acl = target_alloc_node_acl(tpg, initiatorname);
if (!acl) if (!acl)
return NULL; return NULL;
/*
* When allocating a dynamically generated node_acl, go ahead
* and take the extra kref now before returning to the fabric
* driver caller.
*
* Note this reference will be released at session shutdown
* time within transport_free_session() code.
*/
kref_get(&acl->acl_kref);
acl->dynamic_node_acl = 1; acl->dynamic_node_acl = 1;
/* /*
......
...@@ -341,7 +341,6 @@ void __transport_register_session( ...@@ -341,7 +341,6 @@ void __transport_register_session(
&buf[0], PR_REG_ISID_LEN); &buf[0], PR_REG_ISID_LEN);
se_sess->sess_bin_isid = get_unaligned_be64(&buf[0]); se_sess->sess_bin_isid = get_unaligned_be64(&buf[0]);
} }
kref_get(&se_nacl->acl_kref);
spin_lock_irq(&se_nacl->nacl_sess_lock); spin_lock_irq(&se_nacl->nacl_sess_lock);
/* /*
...@@ -432,6 +431,7 @@ void target_put_nacl(struct se_node_acl *nacl) ...@@ -432,6 +431,7 @@ void target_put_nacl(struct se_node_acl *nacl)
{ {
kref_put(&nacl->acl_kref, target_complete_nacl); kref_put(&nacl->acl_kref, target_complete_nacl);
} }
EXPORT_SYMBOL(target_put_nacl);
void transport_deregister_session_configfs(struct se_session *se_sess) void transport_deregister_session_configfs(struct se_session *se_sess)
{ {
...@@ -464,6 +464,15 @@ EXPORT_SYMBOL(transport_deregister_session_configfs); ...@@ -464,6 +464,15 @@ 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;
/*
* 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) { if (se_sess->sess_cmd_map) {
percpu_ida_destroy(&se_sess->sess_tag_pool); percpu_ida_destroy(&se_sess->sess_tag_pool);
kvfree(se_sess->sess_cmd_map); kvfree(se_sess->sess_cmd_map);
...@@ -478,7 +487,7 @@ void transport_deregister_session(struct se_session *se_sess) ...@@ -478,7 +487,7 @@ void transport_deregister_session(struct se_session *se_sess)
const struct target_core_fabric_ops *se_tfo; const struct target_core_fabric_ops *se_tfo;
struct se_node_acl *se_nacl; struct se_node_acl *se_nacl;
unsigned long flags; unsigned long flags;
bool comp_nacl = true, drop_nacl = false; bool drop_nacl = false;
if (!se_tpg) { if (!se_tpg) {
transport_free_session(se_sess); transport_free_session(se_sess);
...@@ -510,18 +519,16 @@ void transport_deregister_session(struct se_session *se_sess) ...@@ -510,18 +519,16 @@ void transport_deregister_session(struct se_session *se_sess)
if (drop_nacl) { if (drop_nacl) {
core_tpg_wait_for_nacl_pr_ref(se_nacl); core_tpg_wait_for_nacl_pr_ref(se_nacl);
core_free_device_list_for_node(se_nacl, se_tpg); core_free_device_list_for_node(se_nacl, se_tpg);
se_sess->se_node_acl = NULL;
kfree(se_nacl); kfree(se_nacl);
comp_nacl = false;
} }
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. * removal context from within transport_free_session() code.
*/ */
if (se_nacl && comp_nacl)
target_put_nacl(se_nacl);
transport_free_session(se_sess); transport_free_session(se_sess);
} }
......
...@@ -169,6 +169,8 @@ void core_allocate_nexus_loss_ua(struct se_node_acl *acl); ...@@ -169,6 +169,8 @@ void core_allocate_nexus_loss_ua(struct se_node_acl *acl);
struct se_node_acl *core_tpg_get_initiator_node_acl(struct se_portal_group *tpg, struct se_node_acl *core_tpg_get_initiator_node_acl(struct se_portal_group *tpg,
unsigned char *); 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 *, struct se_node_acl *core_tpg_check_initiator_node_acl(struct se_portal_group *,
unsigned char *); unsigned char *);
int core_tpg_set_initiator_node_queue_depth(struct se_node_acl *, u32); int core_tpg_set_initiator_node_queue_depth(struct se_node_acl *, u32);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment