Commit 7582207b authored by Md Haris Iqbal's avatar Md Haris Iqbal Committed by Jason Gunthorpe

RDMA/rtrs-clt: Close rtrs client conn before destroying rtrs clt session files

KASAN detected the following BUG:

  BUG: KASAN: use-after-free in rtrs_clt_update_wc_stats+0x41/0x100 [rtrs_client]
  Read of size 8 at addr ffff88bf2fb4adc0 by task swapper/0/0

  CPU: 0 PID: 0 Comm: swapper/0 Tainted: G           O      5.4.84-pserver #5.4.84-1+feature+linux+5.4.y+dbg+20201216.1319+b6b887b~deb10
  Hardware name: Supermicro H8QG6/H8QG6, BIOS 3.00       09/04/2012
  Call Trace:
   <IRQ>
   dump_stack+0x96/0xe0
   print_address_description.constprop.4+0x1f/0x300
   ? irq_work_claim+0x2e/0x50
   __kasan_report.cold.8+0x78/0x92
   ? rtrs_clt_update_wc_stats+0x41/0x100 [rtrs_client]
   kasan_report+0x10/0x20
   rtrs_clt_update_wc_stats+0x41/0x100 [rtrs_client]
   rtrs_clt_rdma_done+0xb1/0x760 [rtrs_client]
   ? lockdep_hardirqs_on+0x1a8/0x290
   ? process_io_rsp+0xb0/0xb0 [rtrs_client]
   ? mlx4_ib_destroy_cq+0x100/0x100 [mlx4_ib]
   ? add_interrupt_randomness+0x1a2/0x340
   __ib_process_cq+0x97/0x100 [ib_core]
   ib_poll_handler+0x41/0xb0 [ib_core]
   irq_poll_softirq+0xe0/0x260
   __do_softirq+0x127/0x672
   irq_exit+0xd1/0xe0
   do_IRQ+0xa3/0x1d0
   common_interrupt+0xf/0xf
   </IRQ>
  RIP: 0010:cpuidle_enter_state+0xea/0x780
  Code: 31 ff e8 99 48 47 ff 80 7c 24 08 00 74 12 9c 58 f6 c4 02 0f 85 53 05 00 00 31 ff e8 b0 6f 53 ff e8 ab 4f 5e ff fb 8b 44 24 04 <85> c0 0f 89 f3 01 00 00 48 8d 7b 14 e8 65 1e 77 ff c7 43 14 00 00
  RSP: 0018:ffffffffab007d58 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffca
  RAX: 0000000000000002 RBX: ffff88b803d69800 RCX: ffffffffa91a8298
  RDX: 0000000000000007 RSI: dffffc0000000000 RDI: ffffffffab021414
  RBP: ffffffffab6329e0 R08: 0000000000000002 R09: 0000000000000000
  R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000002
  R13: 000000bf39d82466 R14: ffffffffab632aa0 R15: ffffffffab632ae0
   ? lockdep_hardirqs_on+0x1a8/0x290
   ? cpuidle_enter_state+0xe5/0x780
   cpuidle_enter+0x3c/0x60
   do_idle+0x2fb/0x390
   ? arch_cpu_idle_exit+0x40/0x40
   ? schedule+0x94/0x120
   cpu_startup_entry+0x19/0x1b
   start_kernel+0x5da/0x61b
   ? thread_stack_cache_init+0x6/0x6
   ? load_ucode_amd_bsp+0x6f/0xc4
   ? init_amd_microcode+0xa6/0xa6
   ? x86_family+0x5/0x20
   ? load_ucode_bsp+0x182/0x1fd
   secondary_startup_64+0xa4/0xb0

  Allocated by task 5730:
   save_stack+0x19/0x80
   __kasan_kmalloc.constprop.9+0xc1/0xd0
   kmem_cache_alloc_trace+0x15b/0x350
   alloc_sess+0xf4/0x570 [rtrs_client]
   rtrs_clt_open+0x3b4/0x780 [rtrs_client]
   find_and_get_or_create_sess+0x649/0x9d0 [rnbd_client]
   rnbd_clt_map_device+0xd7/0xf50 [rnbd_client]
   rnbd_clt_map_device_store+0x4ee/0x970 [rnbd_client]
   kernfs_fop_write+0x141/0x240
   vfs_write+0xf3/0x280
   ksys_write+0xba/0x150
   do_syscall_64+0x68/0x270
   entry_SYSCALL_64_after_hwframe+0x49/0xbe

  Freed by task 5822:
   save_stack+0x19/0x80
   __kasan_slab_free+0x125/0x170
   kfree+0xe7/0x3f0
   kobject_put+0xd3/0x240
   rtrs_clt_destroy_sess_files+0x3f/0x60 [rtrs_client]
   rtrs_clt_close+0x3c/0x80 [rtrs_client]
   close_rtrs+0x45/0x80 [rnbd_client]
   rnbd_client_exit+0x10f/0x2bd [rnbd_client]
   __x64_sys_delete_module+0x27b/0x340
   do_syscall_64+0x68/0x270
   entry_SYSCALL_64_after_hwframe+0x49/0xbe

When rtrs_clt_close is triggered, it iterates over all the present
rtrs_clt_sess and triggers close on them. However, the call to
rtrs_clt_destroy_sess_files is done before the rtrs_clt_close_conns. This
is incorrect since during the initialization phase we allocate
rtrs_clt_sess first, and then we go ahead and create rtrs_clt_con for it.

If we free the rtrs_clt_sess structure before closing the rtrs_clt_con, it
may so happen that an inflight IO completion would trigger the function
rtrs_clt_rdma_done, which would lead to the above UAF case.

Hence close the rtrs_clt_con connections first, and then trigger the
destruction of session files.

Fixes: 6a98d71d ("RDMA/rtrs: client: main functionality")
Link: https://lore.kernel.org/r/20210325153308.1214057-12-gi-oh.kim@ionos.comSigned-off-by: default avatarMd Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: default avatarJack Wang <jinpu.wang@ionos.com>
Signed-off-by: default avatarGioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
parent a5e13c6d
......@@ -2720,8 +2720,8 @@ void rtrs_clt_close(struct rtrs_clt *clt)
/* Now it is safe to iterate over all paths without locks */
list_for_each_entry_safe(sess, tmp, &clt->paths_list, s.entry) {
rtrs_clt_destroy_sess_files(sess, NULL);
rtrs_clt_close_conns(sess, true);
rtrs_clt_destroy_sess_files(sess, NULL);
kobject_put(&sess->kobj);
}
free_clt(clt);
......
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