Re: [RFC-PATCH] nfsd: when unhashing openowners, increment openowner's refcount

Alex Lyakas
2019-08-25T10:12:47Z

Hi Bruce, Chuck,

You are listed as maintainers of nfsd. Can you please take a look at
the below patch?

Thanks,
Alex.

On Wed, Aug 21, 2019 at 7:50 PM Alex Lyakas <alex@zadara.com> wrote:
>
> release_openowner() expects an extra refcnt taken for the openowner,
> which it is releasing.
>
> With nfsd_inject_forget_client_openowners() and nfsd_inject_forget_openowners(),
> we unhash openowners and collect them into a reaplist. Later we call
> nfsd_reap_openowners(), which calls release_openowner(), which releases all openowner's stateids.
> Each OPEN stateid holds a refcnt on the openowner. Therefore, after releasing
> the last OPEN stateid via its sc_free function, which is nfs4_free_ol_stateid,
> nfs4_put_stateowner() will be called, which will realize its the last
> refcnt for the openowner. As a result, openowner will be freed.
> But later, release_openowner() will go ahead and call release_last_closed_stateid()
> and nfs4_put_stateowner() on the same openowner which was just released.
> This corrupts memory and causes random crashes.
>
> After we fixed this, we confirmed that the openowner is not freed
> prematurely. It is freed by release_openowner() final call
> to nfs4_put_stateowner().
>
> However, we still get (other) random crashes and memory corruptions
> when nfsd_inject_forget_client_openowners() and
> nfsd_inject_forget_openowners().
> According to our analysis, we don't see any other refcount issues.
> Can anybody from the community review these flows for other potentials issues?
>
> Signed-off-by: Alex Lyakas <alex@zadara.com>
> ---
> fs/nfsd/nfs4state.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7857942..4e9afca 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -7251,6 +7251,7 @@ static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
> func(oop);
> if (collect) {
> atomic_inc(&clp->cl_rpc_users);
> + nfs4_get_stateowner(&oop->oo_owner);
> list_add(&oop->oo_perclient, collect);
> }
> }
> --
> 1.9.1
>