diff options
author | Bart Van Assche <bvanassche@acm.org> | 2019-09-30 16:17:05 -0700 |
---|---|---|
committer | Jason Gunthorpe <jgg@mellanox.com> | 2019-10-04 15:35:07 -0300 |
commit | be408e65f55ea751ca6e8d08094951c50ce9d3a3 (patch) | |
tree | 0e24285163a58acb517593f220c3073ecbdb8e4d /drivers/infiniband/ulp | |
parent | 6eaed91c673ad5baa0af8a05dc756f3a457468f3 (diff) | |
download | linux-be408e65f55ea751ca6e8d08094951c50ce9d3a3.tar.gz linux-be408e65f55ea751ca6e8d08094951c50ce9d3a3.tar.bz2 linux-be408e65f55ea751ca6e8d08094951c50ce9d3a3.zip |
RDMA/srpt: Rework the code that waits until an RDMA port is no longer in use
The current implementation does not wait until srpt_release_channel()
has finished and hence can trigger a use-after-free. Rework
srpt_release_sport() such that it waits until srpt_release_channel()
has finished. This patch fixes the following KASAN complaint:
==================================================================
BUG: KASAN: use-after-free in srpt_free_ioctx.part.23+0x42/0x100 [ib_srpt]
Read of size 8 at addr ffff888115c71100 by task kworker/4:3/807
CPU: 4 PID: 807 Comm: kworker/4:3 Not tainted 5.3.0-dbg+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
Workqueue: events srpt_release_channel_work [ib_srpt]
Call Trace:
dump_stack+0x86/0xca
print_address_description+0x74/0x32d
__kasan_report.cold.6+0x1b/0x36
kasan_report+0x12/0x17
__asan_load8+0x54/0x90
srpt_free_ioctx.part.23+0x42/0x100 [ib_srpt]
srpt_free_ioctx_ring.part.24+0x50/0x80 [ib_srpt]
srpt_release_channel_work+0x2ad/0x390 [ib_srpt]
process_one_work+0x51a/0xa60
worker_thread+0x67/0x5b0
kthread+0x1dc/0x200
ret_from_fork+0x24/0x30
Allocated by task 984:
save_stack+0x21/0x90
__kasan_kmalloc.constprop.9+0xc7/0xd0
kasan_kmalloc+0x9/0x10
__kmalloc+0x153/0x370
srpt_add_one+0x4f/0x570 [ib_srpt]
add_client_context+0x251/0x290 [ib_core]
ib_register_client+0x1da/0x220 [ib_core]
iblock_get_alignment_offset_lbas+0x6b/0x100 [target_core_iblock]
do_one_initcall+0xcd/0x43a
do_init_module+0x103/0x380
load_module+0x3b77/0x3eb0
__do_sys_finit_module+0x12d/0x1b0
__x64_sys_finit_module+0x43/0x50
do_syscall_64+0x6b/0x2d0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 1128:
save_stack+0x21/0x90
__kasan_slab_free+0x139/0x190
kasan_slab_free+0xe/0x10
slab_free_freelist_hook+0x67/0x1e0
kfree+0xcb/0x2a0
srpt_remove_one+0x569/0x5b0 [ib_srpt]
remove_client_context+0x9a/0xe0 [ib_core]
disable_device+0x106/0x1b0 [ib_core]
__ib_unregister_device+0x5f/0xf0 [ib_core]
ib_unregister_device_and_put+0x48/0x60 [ib_core]
nldev_dellink+0x120/0x180 [ib_core]
rdma_nl_rcv+0x287/0x480 [ib_core]
netlink_unicast+0x2cc/0x370
netlink_sendmsg+0x3b1/0x630
__sys_sendto+0x1db/0x290
__x64_sys_sendto+0x80/0xa0
do_syscall_64+0x6b/0x2d0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff888115c71100
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 0 bytes inside of
4096-byte region [ffff888115c71100, ffff888115c72100)
The buggy address belongs to the page:
page:ffffea0004571c00 refcount:1 mapcount:0 mapping:ffff88811ac0de00 index:0xffff888115c70000 compound_mapcount: 0
flags: 0x2fff000000010200(slab|head)
raw: 2fff000000010200 ffffea00045ac408 ffffea0004593208 ffff88811ac0de00
raw: ffff888115c70000 0000000000070002 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888115c71000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888115c71080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff888115c71100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff888115c71180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888115c71200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Link: https://lore.kernel.org/r/20190930231707.48259-14-bvanassche@acm.org
Cc: Honggang LI <honli@redhat.com>
Cc: Laurence Oberman <loberman@redhat.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Diffstat (limited to 'drivers/infiniband/ulp')
-rw-r--r-- | drivers/infiniband/ulp/srpt/ib_srpt.c | 42 | ||||
-rw-r--r-- | drivers/infiniband/ulp/srpt/ib_srpt.h | 6 |
2 files changed, 25 insertions, 23 deletions
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c index 88e11a250c96..5e402f7b9692 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.c +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c @@ -2023,10 +2023,17 @@ static void srpt_set_enabled(struct srpt_port *sport, bool enabled) __srpt_close_all_ch(sport); } +static void srpt_drop_sport_ref(struct srpt_port *sport) +{ + if (atomic_dec_return(&sport->refcount) == 0 && sport->freed_channels) + complete(sport->freed_channels); +} + static void srpt_free_ch(struct kref *kref) { struct srpt_rdma_ch *ch = container_of(kref, struct srpt_rdma_ch, kref); + srpt_drop_sport_ref(ch->sport); kfree_rcu(ch, rcu); } @@ -2087,8 +2094,6 @@ static void srpt_release_channel_work(struct work_struct *w) kmem_cache_destroy(ch->req_buf_cache); - wake_up(&sport->ch_releaseQ); - kref_put(&ch->kref, srpt_free_ch); } @@ -2307,6 +2312,12 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev, goto destroy_ib; } + /* + * Once a session has been created destruction of srpt_rdma_ch objects + * will decrement sport->refcount. Hence increment sport->refcount now. + */ + atomic_inc(&sport->refcount); + mutex_lock(&sport->mutex); if ((req->req_flags & SRP_MTCH_ACTION) == SRP_MULTICHAN_SINGLE) { @@ -2889,39 +2900,29 @@ static void srpt_refresh_port_work(struct work_struct *work) srpt_refresh_port(sport); } -static bool srpt_ch_list_empty(struct srpt_port *sport) -{ - struct srpt_nexus *nexus; - bool res = true; - - rcu_read_lock(); - list_for_each_entry(nexus, &sport->nexus_list, entry) - if (!list_empty(&nexus->ch_list)) - res = false; - rcu_read_unlock(); - - return res; -} - /** * srpt_release_sport - disable login and wait for associated channels * @sport: SRPT HCA port. */ static int srpt_release_sport(struct srpt_port *sport) { + DECLARE_COMPLETION_ONSTACK(c); struct srpt_nexus *nexus, *next_n; struct srpt_rdma_ch *ch; WARN_ON_ONCE(irqs_disabled()); + sport->freed_channels = &c; + mutex_lock(&sport->mutex); srpt_set_enabled(sport, false); mutex_unlock(&sport->mutex); - while (wait_event_timeout(sport->ch_releaseQ, - srpt_ch_list_empty(sport), 5 * HZ) <= 0) { - pr_info("%s_%d: waiting for session unregistration ...\n", - dev_name(&sport->sdev->device->dev), sport->port); + while (atomic_read(&sport->refcount) > 0 && + wait_for_completion_timeout(&c, 5 * HZ) <= 0) { + pr_info("%s_%d: waiting for unregistration of %d sessions ...\n", + dev_name(&sport->sdev->device->dev), sport->port, + atomic_read(&sport->refcount)); rcu_read_lock(); list_for_each_entry(nexus, &sport->nexus_list, entry) { list_for_each_entry(ch, &nexus->ch_list, list) { @@ -3130,7 +3131,6 @@ static void srpt_add_one(struct ib_device *device) for (i = 1; i <= sdev->device->phys_port_cnt; i++) { sport = &sdev->port[i - 1]; INIT_LIST_HEAD(&sport->nexus_list); - init_waitqueue_head(&sport->ch_releaseQ); mutex_init(&sport->mutex); sport->sdev = sdev; sport->port = i; diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h index d0955bc0343d..f3df791dd877 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.h +++ b/drivers/infiniband/ulp/srpt/ib_srpt.h @@ -381,7 +381,8 @@ struct srpt_port_attrib { * @port_gid_tpg: TPG associated with target port GID. * @port_gid_wwn: WWN associated with target port GID. * @port_attrib: Port attributes that can be accessed through configfs. - * @ch_releaseQ: Enables waiting for removal from nexus_list. + * @refcount: Number of objects associated with this port. + * @freed_channels: Completion that will be signaled once @refcount becomes 0. * @mutex: Protects nexus_list. * @nexus_list: Nexus list. See also srpt_nexus.entry. */ @@ -401,7 +402,8 @@ struct srpt_port { struct se_portal_group port_gid_tpg; struct se_wwn port_gid_wwn; struct srpt_port_attrib port_attrib; - wait_queue_head_t ch_releaseQ; + atomic_t refcount; + struct completion *freed_channels; struct mutex mutex; struct list_head nexus_list; }; |