Re: [PATCH rdma-rc 5/5] RDMA/ucma: Fix memory leak of connection request

[Jason Gunthorpe <jgg@nvidia.com>]

On Sun, Dec 13, 2020 at 03:29:40PM +0200, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg@nvidia.com>
> 
> Add missing call to xa_erase when destroy connection request.
> It fixes the below memory leak.
> 
> unreferenced object 0xffff88812a340490 (size 576):
> comm “kworker/5:0”, pid 96291, jiffies 4296565270 (age 1835.596s)
> hex dump (first 32 bytes):
> 00 20 03 00 00 00 00 00 00 00 00 00 00 00 00 00 . …………..
> a0 d3 1a a0 ff ff ff ff a8 04 34 2a 81 88 ff ff ……….4*….
> backtrace:
> [<0000000059399d4c>] xas_alloc+0x94/0xb0
> [<00000000d855673c>] xas_create+0x1f4/0×4c0
> [<00000000336166d1>] xas_store+0x52/0×5e0
> [<000000006b811da0>] __xa_alloc+0xab/0×140
> [<00000000cf0e9936>] ucma_alloc_ctx+0x197/0×1f0 [rdma_ucm]
> [<000000008f99b6bb>] ucma_event_handler+0x17b/0×2e0 [rdma_ucm]
> [<000000000a07fc34>] cma_cm_event_handler+0x6f/0×390 [rdma_cm]
> [<00000000fe05d574>] cma_ib_req_handler+0x1163/0×2370 [rdma_cm]
> [<000000004516baf4>] cm_work_handler+0xeda/0×2340 [ib_cm]
> [<000000008a83945b>] process_one_work+0x27c/0×610
> [<00000000b71b71e2>] worker_thread+0x2d/0×3c0
> [<00000000caab54ff>] kthread+0x125/0×140
> [<000000004303d699>] ret_from_fork+0x1f/0×30
> 
> Fixes: a1d33b70dbbc ("RDMA/ucma: Rework how new connections are passed through event delivery")
> Signed-off-by: Maor Gottlieb <maorg@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
>  drivers/infiniband/core/ucma.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 7dab9a27a145..b0b9ea90a27d 100644
> +++ b/drivers/infiniband/core/ucma.c
> @@ -549,8 +549,10 @@ static int ucma_free_ctx(struct ucma_context *ctx)
>  	list_for_each_entry_safe(uevent, tmp, &list, list) {
>  		list_del(&uevent->list);
>  		if (uevent->resp.event == RDMA_CM_EVENT_CONNECT_REQUEST &&
> -		    uevent->conn_req_ctx != ctx)
> +		    uevent->conn_req_ctx != ctx) {
> +			xa_erase(&ctx_table, uevent->conn_req_ctx->id);
>  			__destroy_id(uevent->conn_req_ctx);

Oooh, yes this is wrong, but this fix isn't right.

At this point ucma_finish_ctx() has been called so this must be
careful to avoid racing with parallel access from the FD side.

Actually all this destroy stuff still looks subtly wrong, sigh. Let me
try again.

Jason