On Sat, Jun 03, 2023 at 07:28:28AM +1000, NeilBrown wrote:
> From: NeilBrown <neilb@xxxxxxx>
> Date: Sat, 3 Jun 2023 07:14:14 +1000
> Subject: [PATCH] lockd: drop inappropriate svc_get() from locked_get()
>
> The below-mentioned patch was intended to simplify refcounting on the
> svc_serv used by locked. The goal was to only ever have a single
> reference from the single thread. To that end we dropped a call to
> lockd_start_svc() (except when creating thread) which would take a
> reference, and dropped the svc_put(serv) that would drop that reference.
>
> Unfortunately we didn't also remove the svc_get() from
> lockd_create_svc() in the case where the svc_serv already existed.
> So after the patch:
> - on the first call the svc_serv was allocated and the one reference
> was given to the thread, so there are no extra references
> - on subsequent calls svc_get() was called so there is now an extra
> reference.
> This is clearly not consistent.
>
> The inconsistency is also clear in the current code in lockd_get()
> takes *two* references, one on nlmsvc_serv and one by incrementing
> nlmsvc_users. This clearly does not match lockd_put().
>
> So: drop that svc_get() from lockd_get() (which used to be in
> lockd_create_svc().
>
> Reported-by: Ido Schimmel <idosch@xxxxxxxxxx>
> Fixes: b73a2972041b ("lockd: move lockd_start_svc() call into lockd_create_svc()")
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
Thanks for the quick fix. I no longer see the memory leak with this
patch.
Tested-by: Ido Schimmel <idosch@xxxxxxxxxx>
