Re: [PATCH rdma-next] RDMA/restrack: Delay QP deletion till all users are gone

[Leon Romanovsky <leon@kernel.org>]

On Thu, Apr 22, 2021 at 11:29:39AM -0300, Jason Gunthorpe wrote:
> On Wed, Apr 21, 2021 at 08:03:22AM +0300, Leon Romanovsky wrote:
> 
> > I didn't understand when reviewed either, but decided to post it anyway
> > to get possible explanation for this RDMA_RESTRACK_MR || RDMA_RESTRACK_QP
> > check.
> 
> I think the whole thing should look more like this and we delete the
> if entirely.

Finally, I had a time to work on it and got immediate reminder why we
have "if ...".

> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 2b9ffc21cbc4ad..479b16b8f6723a 100644

<...>

> @@ -1963,31 +1969,33 @@ int ib_destroy_qp_user(struct ib_qp *qp, struct ib_udata *udata)
>  		rdma_rw_cleanup_mrs(qp);
>  
>  	rdma_counter_unbind_qp(qp, true);
> -	rdma_restrack_del(&qp->res);
> +	rdma_restrack_prepare_del(&qp->res);
>  	ret = qp->device->ops.destroy_qp(qp, udata);
> -	if (!ret) {
> -		if (alt_path_sgid_attr)
> -			rdma_put_gid_attr(alt_path_sgid_attr);
> -		if (av_sgid_attr)
> -			rdma_put_gid_attr(av_sgid_attr);
> -		if (pd)
> -			atomic_dec(&pd->usecnt);
> -		if (scq)
> -			atomic_dec(&scq->usecnt);
> -		if (rcq)
> -			atomic_dec(&rcq->usecnt);
> -		if (srq)
> -			atomic_dec(&srq->usecnt);
> -		if (ind_tbl)
> -			atomic_dec(&ind_tbl->usecnt);
> -		if (sec)
> -			ib_destroy_qp_security_end(sec);
> -	} else {
> +	if (ret) {
> +		rdma_restrack_abort_del(&qp->res);
>  		if (sec)
>  			ib_destroy_qp_security_abort(sec);
> +		return ret;
>  	}
>  
> -	return ret;
> +	rdma_restrack_finish_del(&qp->res);

This causes to use-after-free because "qp" is released in the driver.

[leonro@vm ~]$ mkt test
.s....[   24.350657] ==================================================================
[   24.350917] BUG: KASAN: use-after-free in rdma_restrack_finish_del+0x1a7/0x210 [ib_core]
[   24.351303] Read of size 1 at addr ffff88800c59f118 by task python3/243
[   24.351416] 
[   24.351495] CPU: 4 PID: 243 Comm: python3 Not tainted 5.12.0-rc2+ #2923
[   24.351644] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
[   24.351832] Call Trace:
[   24.351886]  dump_stack+0x93/0xc2
[   24.351965]  print_address_description.constprop.0+0x18/0x140
[   24.352074]  ? rdma_restrack_finish_del+0x1a7/0x210 [ib_core]
[   24.352204]  ? rdma_restrack_finish_del+0x1a7/0x210 [ib_core]
[   24.352333]  kasan_report.cold+0x7c/0xd8
[   24.352413]  ? rdma_restrack_finish_del+0x1a7/0x210 [ib_core]
[   24.352543]  rdma_restrack_finish_del+0x1a7/0x210 [ib_core]
[   24.352654]  ib_destroy_qp_user+0x297/0x540 [ib_core]
[   24.352769]  uverbs_free_qp+0x7d/0x150 [ib_uverbs]
[   24.352864]  uverbs_destroy_uobject+0x164/0x6c0 [ib_uverbs]
[   24.352949]  uobj_destroy+0x72/0xf0 [ib_uverbs]
[   24.353041]  ib_uverbs_cmd_verbs+0x19b9/0x2ee0 [ib_uverbs]
[   24.353147]  ? uverbs_free_qp+0x150/0x150 [ib_uverbs]
[   24.353246]  ? lockdep_hardirqs_on_prepare+0x3e0/0x3e0
[   24.353340]  ? uverbs_fill_udata+0x540/0x540 [ib_uverbs]
[   24.353455]  ? filemap_map_pages+0x2d9/0xf30
[   24.353552]  ? lock_acquire+0x1a9/0x6d0
[   24.353620]  ? ib_uverbs_ioctl+0x11e/0x260 [ib_uverbs]
[   24.353720]  ? __might_fault+0xba/0x160
[   24.353790]  ? lock_release+0x6c0/0x6c0
[   24.353866]  ib_uverbs_ioctl+0x169/0x260 [ib_uverbs]
[   24.353958]  ? ib_uverbs_ioctl+0x11e/0x260 [ib_uverbs]
[   24.354049]  ? ib_uverbs_cmd_verbs+0x2ee0/0x2ee0 [ib_uverbs]
[   24.354158]  __x64_sys_ioctl+0x7e6/0x1050
[   24.354217]  ? generic_block_fiemap+0x60/0x60
[   24.354305]  ? down_write_nested+0x150/0x150
[   24.354400]  ? do_user_addr_fault+0x219/0xdc0
[   24.354497]  ? lockdep_hardirqs_on_prepare+0x273/0x3e0
[   24.354584]  ? syscall_enter_from_user_mode+0x1d/0x50
[   24.354678]  do_syscall_64+0x2d/0x40
[   24.354747]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[   24.354837] RIP: 0033:0x7fbfd862b5db
[   24.354911] Code: 89 d8 49 8d 3c 1c 48 f7 d8 49 39 c4 72 b5 e8 1c ff ff ff 85 c0 78 ba 4c 89 e0 5b 5d 41 5c c3 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 6d b8 0c 00 f7 d8 64 89 01 48
[   24.355200] RSP: 002b:00007fff7e9fde58 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[   24.355336] RAX: ffffffffffffffda RBX: 00007fff7e9fdf88 RCX: 00007fbfd862b5db
[   24.355468] RDX: 00007fff7e9fdf70 RSI: 00000000c0181b01 RDI: 0000000000000005
[   24.355587] RBP: 00007fff7e9fdf50 R08: 000055d6a4f8b3f0 R09: 00007fbfd800d000
[   24.355713] R10: 00007fbfd800d0f0 R11: 0000000000000246 R12: 00007fff7e9fdf50
[   24.355836] R13: 00007fff7e9fdf2c R14: 000055d6a4e59d90 R15: 000055d6a4f8b530
[   24.355973] 

So let's finish memory allocation conversion patches before.

Thanks