Re: [PATCH] rdma: fix cm kref leak caused by race in dtr_disconnect_path
Philipp Reisner <[email protected]> Fri, 5 Jun 2026 14:35:02 +0200
| Newsgroups | dev.linux.lists.drbd-dev |
|---|---|
| Message-ID | <CADGDV=VPu-ePxUzTeNMTssbBPRDVu-VAh9fAEkP=XNNw_YeV4g@mail.gmail.com> |
Thanks, merged. On Wed, May 13, 2026 at 8:12=E2=80=AFAM zhengbing.huang <[email protected]> wrote: > > When peer's RoCE NIC goes down unexpectedly, a race condition can occur > in dtr_disconnect_path() between __dtr_disconnect_path() and the > subsequent xchg(&path->cm, NULL). If another thread installs a new cm > via dtr_path_prepare() after __dtr_disconnect_path() returns (or sees > path->cm as NULL and returns early), the xchg will take the new cm > whose QP was never moved to ERR state. This leaves posted rx_descs > stuck in the receive queue with no flush CQEs, preventing their kref > references on cm from being released, causing a cm kref leak. > > Fix by ensuring ib_modify_qp(ERR) is always executed on the cm obtained > via xchg in dtr_disconnect_path() and dtr_free(), so that posted WRs > are flushed regardless of whether __dtr_disconnect_path() successfully > processed that cm. Extract the QP->ERR logic into __dtr_modify_qp_to_err(= ) > helper for reuse across __dtr_disconnect_path, dtr_disconnect_path, > dtr_remove_cm_from_path, and dtr_free. > > Signed-off-by: zhengbing.huang <[email protected]> > --- > drbd/drbd_transport_rdma.c | 48 +++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 21 deletions(-) > > diff --git a/drbd/drbd_transport_rdma.c b/drbd/drbd_transport_rdma.c > index 195e45648..9099fee93 100644 > --- a/drbd/drbd_transport_rdma.c > +++ b/drbd/drbd_transport_rdma.c > @@ -346,6 +346,7 @@ static void dtr_free_tx_desc(struct dtr_cm *cm, struc= t dtr_tx_desc *tx_desc); > static void dtr_free_rx_desc(struct dtr_rx_desc *rx_desc); > static void dtr_cma_disconnect_work_fn(struct work_struct *work); > static void dtr_disconnect_path(struct dtr_path *path); > +static void __dtr_modify_qp_to_err(struct dtr_cm *cm); > static void __dtr_disconnect_path(struct dtr_path *path); > static int dtr_init_flow(struct dtr_path *path, enum drbd_stream stream)= ; > static int dtr_cm_alloc_rdma_res(struct dtr_cm *cm); > @@ -507,8 +508,10 @@ static void dtr_free(struct drbd_transport *transpor= t, enum drbd_tr_free_op free > struct dtr_cm *cm; > > cm =3D xchg(&path->cm, NULL); // RCU xchg > - if (cm) > + if (cm) { > + __dtr_modify_qp_to_err(cm); > kref_put(&cm->kref, dtr_destroy_cm); > + } > } > > timer_delete_sync(&rdma_transport->control_timer); > @@ -1091,15 +1094,8 @@ static void dtr_remove_cm_from_path(struct dtr_pat= h *path, struct dtr_cm *failed > struct dtr_cm *cm; > > cm =3D cmpxchg(&path->cm, failed_cm, NULL); // RCU &path->cm > - if (cm =3D=3D failed_cm && cm->id && cm->id->qp) { > - struct drbd_transport *transport =3D path->path.transport= ; > - struct ib_qp_attr attr =3D { .qp_state =3D IB_QPS_ERR }; > - int err; > - > - err =3D ib_modify_qp(cm->id->qp, &attr, IB_QP_STATE); > - if (err) > - tr_err(transport, "ib_modify_qp failed %d\n", err= ); > - > + if (cm =3D=3D failed_cm) { > + __dtr_modify_qp_to_err(cm); > kref_put(&cm->kref, dtr_destroy_cm); > } > } > @@ -2667,9 +2663,25 @@ static void dtr_end_tx_work_fn(struct work_struct = *work) > kref_put(&cm->kref, dtr_destroy_cm); > } > > -static void __dtr_disconnect_path(struct dtr_path *path) > +static void __dtr_modify_qp_to_err(struct dtr_cm *cm) > { > + struct dtr_path *path =3D cm->path; > + struct drbd_transport *transport =3D path->path.transport; > struct ib_qp_attr attr =3D { .qp_state =3D IB_QPS_ERR }; > + int err; > + > + /* between dtr_alloc_cm() and dtr_cm_alloc_rdma_res() cm->id->qp = is NULL */ > + if (!cm->id || !cm->id->qp) > + return; > + > + /* With putting the QP into error state, it has to hand back all = posted rx_descs */ > + err =3D ib_modify_qp(cm->id->qp, &attr, IB_QP_STATE); > + if (err) > + tr_err(transport, "ib_modify_qp failed %d\n", err); > +} > + > +static void __dtr_disconnect_path(struct dtr_path *path) > +{ > struct drbd_transport *transport; > enum connect_state_enum a, p; > bool was_scheduled; > @@ -2741,15 +2753,7 @@ static void __dtr_disconnect_path(struct dtr_path = *path) > cm->state); > > out: > - /* between dtr_alloc_cm() and dtr_cm_alloc_rdma_res() cm->id->qp = is NULL */ > - if (cm->id->qp) { > - /* With putting the QP into error state, it has to hand b= ack > - all posted rx_descs */ > - err =3D ib_modify_qp(cm->id->qp, &attr, IB_QP_STATE); > - if (err) > - tr_err(transport, "ib_modify_qp failed %d\n", err= ); > - } > - > + __dtr_modify_qp_to_err(cm); > /* > * We are expecting one of RDMA_CM_EVENT_ESTABLISHED, _UNREACHABL= E, > * _CONNECT_ERROR, or _REJECTED on this cm. Some RDMA drivers rep= ort > @@ -2836,8 +2840,10 @@ static void dtr_disconnect_path(struct dtr_path *p= ath) > cancel_work_sync(&path->refill_rx_descs_work); > > cm =3D xchg(&path->cm, NULL); // RCU xchg > - if (cm) > + if (cm) { > + __dtr_modify_qp_to_err(cm); > kref_put(&cm->kref, dtr_destroy_cm); > + } > } > > static void dtr_destroy_listener(struct drbd_listener *generic_listener) > -- > 2.43.0 >