UCT/IB/RDMACM: Take async block around unprotected librdmacm calls

The cm event handler runs under the cm's async block (held by
ucs_async_handler_dispatch via context_try_block). rdma_destroy_id and
related librdmacm state-mutating calls must take the same block to
serialize with rdma_get_cm_event() running in the handler; otherwise a
concurrent destroy can remove the cm_id's userspace tracking while the
handler is mid-lookup inside librdmacm, producing a NULL deref at
pthread_mutex_lock(&id_priv->mut) inside rdma_get_cm_event().

The ep destructor and the listener destructor already take the block.
Four other call sites did not:

  - rdmacm_listener.c listener init error path (rdma_destroy_id on
    rdma_bind_addr / rdma_listen failure)
  - rdmacm_listener.c uct_rdmacm_listener_reject (rdma_reject +
    rdma_destroy_id + rdma_ack_cm_event on a connect_request)
  - rdmacm_cm_ep.c client init error path (rdma_destroy_id on
    rdma_resolve_addr failure)
  - rdmacm_cm_ep.c server init (rdma_ack_cm_event + rdma_migrate_id,
    plus rdma_destroy_id on the error path; the success-path
    server_send_priv_data already takes the same (recursive) block)

Observed as a SIGSEGV inside librdmacm during multi-threaded sockaddr
gtests (test_ucp_sockaddr_destroy_ep_on_err, test_ucp_sockaddr_wireup_fail)
where ep teardown and connect_request rejection interleave with event
delivery on the same channel.

Signed-off-by: NirWolfer <nwolfer@nvidia.com>

Author: NirWolfer

src/uct/ib/rdmacm/rdmacm_cm_ep.c

@@ -522,7 +522,9 @@ static ucs_status_t uct_rdmacm_cm_ep_client_init(uct_rdmacm_cm_ep_t *cep,
     return UCS_OK;
 
 err_destroy_id:
+    UCS_ASYNC_BLOCK(uct_rdmacm_cm_ep_get_async(cep));
     uct_rdmacm_cm_destroy_id(cep->id);
+    UCS_ASYNC_UNBLOCK(uct_rdmacm_cm_ep_get_async(cep));
 err:
     return status;
 }
@@ -587,6 +589,11 @@ static ucs_status_t uct_rdmacm_cm_ep_server_init(uct_rdmacm_cm_ep_t *cep,
     ucs_status_t status;
     char ep_str[UCT_RDMACM_EP_STRING_LEN];
 
+    /* Serialize the librdmacm ack/migrate/destroy calls on this channel
+     * against the cm event handler. server_send_priv_data takes the same
+     * (recursive) block on the success path. */
+    UCS_ASYNC_BLOCK(uct_rdmacm_cm_ep_get_async(cep));
+
     status = uct_rdmacm_cm_ack_event(event);
     if (status != UCS_OK) {
         goto err_destroy_id;
@@ -621,12 +628,14 @@ static ucs_status_t uct_rdmacm_cm_ep_server_init(uct_rdmacm_cm_ep_t *cep,
                   id, listen_ev_ch, cm, cm->ev_ch);
     }
 
+    UCS_ASYNC_UNBLOCK(uct_rdmacm_cm_ep_get_async(cep));
     return uct_rdmacm_cm_ep_server_send_priv_data(cep, params);
 
 err:
     cep->id = NULL;
 err_destroy_id:
     uct_rdmacm_cm_destroy_id(id);
+    UCS_ASYNC_UNBLOCK(uct_rdmacm_cm_ep_get_async(cep));
     return status;
 }
 

src/uct/ib/rdmacm/rdmacm_listener.c

@@ -106,7 +106,9 @@ UCS_CLASS_INIT_FUNC(uct_rdmacm_listener_t, uct_cm_h cm,
     return UCS_OK;
 
 err_destroy_id:
+    UCS_ASYNC_BLOCK(uct_rdmacm_cm_get_async(rdmacm_cm));
     uct_rdmacm_cm_destroy_id(self->id);
+    UCS_ASYNC_UNBLOCK(uct_rdmacm_cm_get_async(rdmacm_cm));
 err:
     return status;
 }
@@ -119,12 +121,19 @@ ucs_status_t uct_rdmacm_listener_reject(uct_listener_h listener,
     uct_rdmacm_cm_t *rdmacm_cm             = ucs_derived_of(listener->cm,
                                                             uct_rdmacm_cm_t);
     struct rdma_cm_event *event            = (struct rdma_cm_event*)conn_request;
+    ucs_status_t status;
 
     ucs_assert_always(rdmacm_listener->id == event->listen_id);
 
+    /* Serialize against the cm event handler, which holds the same async
+     * block when calling rdma_get_cm_event() on this channel. */
+    UCS_ASYNC_BLOCK(uct_rdmacm_cm_get_async(rdmacm_cm));
     uct_rdmacm_cm_reject(rdmacm_cm, event->id);
     uct_rdmacm_cm_destroy_id(event->id);
-    return uct_rdmacm_cm_ack_event(event);
+    status = uct_rdmacm_cm_ack_event(event);
+    UCS_ASYNC_UNBLOCK(uct_rdmacm_cm_get_async(rdmacm_cm));
+
+    return status;
 }
 
 UCS_CLASS_CLEANUP_FUNC(uct_rdmacm_listener_t)