Re:Re: [PATCH 03/11] drbd_transport_rdma: put kref for cm in dtr_path_established in error path

[Zhengbing]

Hi Philipp,

Thank you for your reply.

We'll get to know those patches.



Best regards,
zhengbing




From: Philipp Reisner <philipp.reisner at linbit.com>
Date: 2024-10-17 00:44:40
To:  Dongsheng Yang <dongsheng.yang at easystack.cn>
Cc:  "zhengbing.huang" <zhengbing.huang at easystack.cn>,drbd-dev at lists.linbit.com,dongsheng.yang at linux.dev
Subject: Re: [PATCH 03/11] drbd_transport_rdma: put kref for cm in dtr_path_established in error path>Hello easystack team,
>
>I probably fixed the (or some of the) problems you intended to fix
>with this patch series.
>
>Please see these recent commits on the master branch:
>
>3f82aed6e drbd_transport: fix a use after free in drbd_get_listener()
>1f2f11c47 rdma: remove dtr_path_established()
>e38120f3d rdma: Fix leaking a cm_id object
>e0f5e307e rdma: fix superficial kref_put() in error code path in
>dtr_cma_accept()
>35a6b002c rdma: fix concurrency of activate_path() and __dtr_disconnect_path()
>6ccd39432 rdma: fix an access after free() in dtr_destroy_cm()
>5a711b347 rdma: fix free() of scheduled delayed_work
>847aab659 rdma: remove misguided kref_get()/kref_put()
>
>I will go over your patch series and your comments. I'm sorry I
>dropped the ball on this. I was out on vacation in July, and when I
>came back, 1000 things needed my attention.
>
>Best regards,
> Philipp
>
>On Mon, Jul 1, 2024 at 4:48 AM Dongsheng Yang
><dongsheng.yang at easystack.cn> wrote:
>>
>>
>>
>> 在 2024/7/1 星期一 上午 10:07, Dongsheng Yang 写道:
>> >
>> >
>> > 在 2024/6/28 星期五 下午 5:40, Philipp Reisner 写道:
>> >> Hello Dongsheng,
>> >>
>> >> Please add more information why you think this change fixes a bug.
>> >> Have you experienced a leak of cm structs?
>> >> We got a RDMA_CM_EVENT_ESTABLISHED event. Even if DRBD does not do
>> >> anything with this cm, we sill expect a RDMA_CM_EVENT_DISCONNECTED in
>> >> the future. Is a problem in the handling of the disconnect?
>> >
>> > If dtr_path_established() go into this branch, it will not
>> > schedule_work(&cm->establish_work);
>> >
>> > That means path->cm->state = DSM_CONNECTED; will not be done in
>> > dtr_path_established_work_fn(), so __dtr_disconnect_path() will not call
>> > rdma_disconnect(). That means this reference will never be put.
>>
>> let me consider this  example:
>> a) rdma_connect() called and RDMA_CM_EVENT_ESTABLISHED received.
>>
>> b) network failure and dtr_path_established() go into error path.
>>
>> c) establish_work will not be scheduled.
>>
>> d) drbdadm down test will hang because cm ref is not put.
>> >>
>> >> best regards,
>> >>   Philipp
>> >>
>> >> On Mon, Jun 24, 2024 at 9:28 AM zhengbing.huang
>> >> <zhengbing.huang at easystack.cn> wrote:
>> >>>
>> >>> From: Dongsheng Yang <dongsheng.yang at easystack.cn>
>> >>>
>> >>> Signed-off-by: Dongsheng Yang <dongsheng.yang at easystack.cn>
>> >>> ---
>> >>>   drbd/drbd_transport_rdma.c | 1 +
>> >>>   1 file changed, 1 insertion(+)
>> >>>
>> >>> diff --git a/drbd/drbd_transport_rdma.c b/drbd/drbd_transport_rdma.c
>> >>> index cfbae0e78..eccd0c6ce 100644
>> >>> --- a/drbd/drbd_transport_rdma.c
>> >>> +++ b/drbd/drbd_transport_rdma.c
>> >>> @@ -922,6 +922,7 @@ static void dtr_path_established(struct dtr_cm *cm)
>> >>>                          atomic_set(&cs->active_state, PCS_INACTIVE);
>> >>>                          wake_up(&cs->wq);
>> >>>                  }
>> >>> +               kref_put(&cm->kref, dtr_destroy_cm);
>> >>>                  return;
>> >>>          }
>> >>>
>> >>> --
>> >>> 2.27.0
>> >>>




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linbit.com/pipermail/drbd-dev/attachments/20241017/152c6da1/attachment.htm>