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
>