scsi: target/core: Make sure that target_wait_for_sess_cmds() waits long 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 <nab@linux-iscsi.org>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: David Disseldorp <ddiss@suse.de>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 0c8eca6ae30fb9c900b88dfdc8a45c44661bdd25..b0416978ab99617c1ba548be8caae6cd4f0646a8 100644 (file)
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -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);
 
@@ -247,6 +256,7 @@ 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) {
@@ -254,7 +264,11 @@ struct se_session *transport_alloc_session(enum target_prot_op sup_prot_ops)
                                " 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;
@@ -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);
@@ -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);
 
@@ -2746,8 +2762,6 @@ 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);
        }
 
@@ -2755,6 +2769,8 @@ static void target_release_cmd_kref(struct kref *kref)
        se_cmd->se_tfo->release_cmd(se_cmd);
        if (compl)
                complete(compl);
+
+       percpu_ref_put(&se_sess->cmd_count);
 }
 
 /**
@@ -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);
 
@@ -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);
 

diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c
index f4afb4b306b01d83aac282e0f4c7a5c329fef759..c2e1fc927fdff8610d831d0e128ceeaf2fad3162 100644 (file)
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -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");
@@ -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;

diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index bb965a35518761e8b84f0b86d095941de0a91522..6e34ec4df6b7cc3ff5c2f28ecaf6da880f5d198d 100644 (file)
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -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;

diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 0a1595f3c5a17cb552aacf950dcce45a0ac14d7a..3b81a5b014977265653a54d0baa8bd65669563b5 100644 (file)
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -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);