Skip to content
This repository has been archived by the owner on Sep 24, 2020. It is now read-only.

Commit

Permalink
scsi: target/core: Make sure that target_wait_for_sess_cmds() waits l…
Browse files Browse the repository at this point in the history 
…ong enough

A session must only be released after all code that accesses the session
structure has finished. Make sure that this is the case by introducing a
new command counter per session that is only decremented after the
.release_cmd() callback has finished. This patch fixes the following crash:

BUG: KASAN: use-after-free in do_raw_spin_lock+0x1c/0x130
Read of size 4 at addr ffff8801534b16e4 by task rmdir/14805
CPU: 16 PID: 14805 Comm: rmdir Not tainted 4.18.0-rc2-dbg+ #5
Call Trace:
dump_stack+0xa4/0xf5
print_address_description+0x6f/0x270
kasan_report+0x241/0x360
__asan_load4+0x78/0x80
do_raw_spin_lock+0x1c/0x130
_raw_spin_lock_irqsave+0x52/0x60
srpt_set_ch_state+0x27/0x70 [ib_srpt]
srpt_disconnect_ch+0x1b/0xc0 [ib_srpt]
srpt_close_session+0xa8/0x260 [ib_srpt]
target_shutdown_sessions+0x170/0x180 [target_core_mod]
core_tpg_del_initiator_node_acl+0xf3/0x200 [target_core_mod]
target_fabric_nacl_base_release+0x25/0x30 [target_core_mod]
config_item_release+0x9c/0x110 [configfs]
config_item_put+0x26/0x30 [configfs]
configfs_rmdir+0x3b8/0x510 [configfs]
vfs_rmdir+0xb3/0x1e0
do_rmdir+0x262/0x2c0
do_syscall_64+0x77/0x230
entry_SYSCALL_64_after_hwframe+0x49/0xbe

Cc: Nicholas Bellinger <[email protected]>
Cc: Mike Christie <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: David Disseldorp <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
  • Loading branch information
@bvanassche @martinkpetersen
bvanassche authored and martinkpetersen committed Dec 8, 2018
1 parent a95be38 commit ad66950
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 12 deletions.
35 changes: 25 additions & 10 deletions drivers/target/target_core_transport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,19 +224,28 @@ void transport_subsystem_check_init(void)
sub_api_initialized = 1;
}

static void target_release_sess_cmd_refcnt(struct percpu_ref *ref)
{
struct se_session *sess = container_of(ref, typeof(*sess), cmd_count);

wake_up(&sess->cmd_list_wq);
}

/**
* transport_init_session - initialize a session object
* @se_sess: Session object pointer.
*
* The caller must have zero-initialized @se_sess before calling this function.
*/
void transport_init_session(struct se_session *se_sess)
int transport_init_session(struct se_session *se_sess)
{
INIT_LIST_HEAD(&se_sess->sess_list);
INIT_LIST_HEAD(&se_sess->sess_acl_list);
INIT_LIST_HEAD(&se_sess->sess_cmd_list);
spin_lock_init(&se_sess->sess_cmd_lock);
init_waitqueue_head(&se_sess->cmd_list_wq);
return percpu_ref_init(&se_sess->cmd_count,
target_release_sess_cmd_refcnt, 0, GFP_KERNEL);
}
EXPORT_SYMBOL(transport_init_session);

Expand All @@ -247,14 +256,19 @@ EXPORT_SYMBOL(transport_init_session);
struct se_session *transport_alloc_session(enum target_prot_op sup_prot_ops)
{
struct se_session *se_sess;
int ret;

se_sess = kmem_cache_zalloc(se_sess_cache, GFP_KERNEL);
if (!se_sess) {
pr_err("Unable to allocate struct se_session from"
" se_sess_cache\n");
return ERR_PTR(-ENOMEM);
}
transport_init_session(se_sess);
ret = transport_init_session(se_sess);
if (ret < 0) {
kfree(se_sess);
return ERR_PTR(ret);
}
se_sess->sup_prot_ops = sup_prot_ops;

return se_sess;
Expand Down Expand Up @@ -578,6 +592,7 @@ void transport_free_session(struct se_session *se_sess)
sbitmap_queue_free(&se_sess->sess_tag_pool);
kvfree(se_sess->sess_cmd_map);
}
percpu_ref_exit(&se_sess->cmd_count);
kmem_cache_free(se_sess_cache, se_sess);
}
EXPORT_SYMBOL(transport_free_session);
Expand Down Expand Up @@ -2716,6 +2731,7 @@ int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref)
}
se_cmd->transport_state |= CMD_T_PRE_EXECUTE;
list_add_tail(&se_cmd->se_cmd_list, &se_sess->sess_cmd_list);
percpu_ref_get(&se_sess->cmd_count);
out:
spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);

Expand Down Expand Up @@ -2746,15 +2762,15 @@ static void target_release_cmd_kref(struct kref *kref)
if (se_sess) {
spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
list_del_init(&se_cmd->se_cmd_list);
if (se_sess->sess_tearing_down && list_empty(&se_sess->sess_cmd_list))
wake_up(&se_sess->cmd_list_wq);
spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
}

target_free_cmd_mem(se_cmd);
se_cmd->se_tfo->release_cmd(se_cmd);
if (compl)
complete(compl);

percpu_ref_put(&se_sess->cmd_count);
}

/**
Expand Down Expand Up @@ -2883,6 +2899,8 @@ void target_sess_cmd_list_set_waiting(struct se_session *se_sess)
spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
se_sess->sess_tearing_down = 1;
spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);

percpu_ref_kill(&se_sess->cmd_count);
}
EXPORT_SYMBOL(target_sess_cmd_list_set_waiting);

Expand All @@ -2897,17 +2915,14 @@ void target_wait_for_sess_cmds(struct se_session *se_sess)

WARN_ON_ONCE(!se_sess->sess_tearing_down);

spin_lock_irq(&se_sess->sess_cmd_lock);
do {
ret = wait_event_lock_irq_timeout(
se_sess->cmd_list_wq,
list_empty(&se_sess->sess_cmd_list),
se_sess->sess_cmd_lock, 180 * HZ);
ret = wait_event_timeout(se_sess->cmd_list_wq,
percpu_ref_is_zero(&se_sess->cmd_count),
180 * HZ);
list_for_each_entry(cmd, &se_sess->sess_cmd_list, se_cmd_list)
target_show_cmd("session shutdown: still waiting for ",
cmd);
} while (ret <= 0);
spin_unlock_irq(&se_sess->sess_cmd_lock);
}
EXPORT_SYMBOL(target_wait_for_sess_cmds);

Expand Down
6 changes: 5 additions & 1 deletion drivers/target/target_core_xcopy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,8 @@ static const struct target_core_fabric_ops xcopy_pt_tfo = {

int target_xcopy_setup_pt(void)
{
int ret;

xcopy_wq = alloc_workqueue("xcopy_wq", WQ_MEM_RECLAIM, 0);
if (!xcopy_wq) {
pr_err("Unable to allocate xcopy_wq\n");
Expand All @@ -491,7 +493,9 @@ int target_xcopy_setup_pt(void)
INIT_LIST_HEAD(&xcopy_pt_nacl.acl_list);
INIT_LIST_HEAD(&xcopy_pt_nacl.acl_sess_list);
memset(&xcopy_pt_sess, 0, sizeof(struct se_session));
transport_init_session(&xcopy_pt_sess);
ret = transport_init_session(&xcopy_pt_sess);
if (ret < 0)
return ret;

xcopy_pt_nacl.se_tpg = &xcopy_pt_tpg;
xcopy_pt_nacl.nacl_sess = &xcopy_pt_sess;
Expand Down
1 change: 1 addition & 0 deletions include/target/target_core_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,7 @@ struct se_session {
struct se_node_acl *se_node_acl;
struct se_portal_group *se_tpg;
void *fabric_sess_ptr;
struct percpu_ref cmd_count;
struct list_head sess_list;
struct list_head sess_acl_list;
struct list_head sess_cmd_list;
Expand Down
2 changes: 1 addition & 1 deletion include/target/target_core_fabric.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ struct se_session *target_setup_session(struct se_portal_group *,
struct se_session *, void *));
void target_remove_session(struct se_session *);

void transport_init_session(struct se_session *);
int transport_init_session(struct se_session *se_sess);
struct se_session *transport_alloc_session(enum target_prot_op);
int transport_alloc_session_tags(struct se_session *, unsigned int,
unsigned int);
Expand Down

0 comments on commit ad66950

Please sign in to comment.