| Message ID | 20240904123457.3027627-1-lilingfeng3@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | nfs: fix memory leak in error path of nfs4_do_reclaim | expand |
On Wed, 2024-09-04 at 20:34 +0800, Li Lingfeng wrote: > Commit c77e22834ae9 ("NFSv4: Fix a potential sleep while atomic in > nfs4_do_reclaim()") separate out the freeing of the state owners from > nfs4_purge_state_owners() and finish it outside the rcu lock. > However, the error path is omitted. As a result, the state owners in > "freeme" will not be released. > Fix it by adding freeing in the error path. > > Fixes: c77e22834ae9 ("NFSv4: Fix a potential sleep while atomic in > nfs4_do_reclaim()") > Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> > --- > fs/nfs/nfs4state.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > index 877f682b45f2..30aba1dedaba 100644 > --- a/fs/nfs/nfs4state.c > +++ b/fs/nfs/nfs4state.c > @@ -1957,6 +1957,7 @@ static int nfs4_do_reclaim(struct nfs_client > *clp, const struct nfs4_state_recov > set_bit(ops->owner_flag_bit, &sp- > >so_flags); > nfs4_put_state_owner(sp); > status = > nfs4_recovery_handle_error(clp, status); > + nfs4_free_state_owners(&freeme); > return (status != 0) ? status : - > EAGAIN; > } > Nice catch! Yes, that leak has been there for quite a while. Thanks for finding it.
On Wed, Sep 4, 2024 at 10:39 AM Trond Myklebust <trondmy@kernel.org> wrote: > > On Wed, 2024-09-04 at 20:34 +0800, Li Lingfeng wrote: > > Commit c77e22834ae9 ("NFSv4: Fix a potential sleep while atomic in > > nfs4_do_reclaim()") separate out the freeing of the state owners from > > nfs4_purge_state_owners() and finish it outside the rcu lock. > > However, the error path is omitted. As a result, the state owners in > > "freeme" will not be released. > > Fix it by adding freeing in the error path. > > > > Fixes: c77e22834ae9 ("NFSv4: Fix a potential sleep while atomic in > > nfs4_do_reclaim()") > > Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> > > --- > > fs/nfs/nfs4state.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > > index 877f682b45f2..30aba1dedaba 100644 > > --- a/fs/nfs/nfs4state.c > > +++ b/fs/nfs/nfs4state.c > > @@ -1957,6 +1957,7 @@ static int nfs4_do_reclaim(struct nfs_client > > *clp, const struct nfs4_state_recov > > set_bit(ops->owner_flag_bit, &sp- > > >so_flags); > > nfs4_put_state_owner(sp); > > status = > > nfs4_recovery_handle_error(clp, status); > > + nfs4_free_state_owners(&freeme); > > return (status != 0) ? status : - > > EAGAIN; > > } > > > > Nice catch! Yes, that leak has been there for quite a while. Thanks for > finding it. Looks good to me, too. I've applied this for v6.12 (with a cc stable so it gets backported) Anna > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > >
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index 877f682b45f2..30aba1dedaba 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -1957,6 +1957,7 @@ static int nfs4_do_reclaim(struct nfs_client *clp, const struct nfs4_state_recov set_bit(ops->owner_flag_bit, &sp->so_flags); nfs4_put_state_owner(sp); status = nfs4_recovery_handle_error(clp, status); + nfs4_free_state_owners(&freeme); return (status != 0) ? status : -EAGAIN; }
Commit c77e22834ae9 ("NFSv4: Fix a potential sleep while atomic in nfs4_do_reclaim()") separate out the freeing of the state owners from nfs4_purge_state_owners() and finish it outside the rcu lock. However, the error path is omitted. As a result, the state owners in "freeme" will not be released. Fix it by adding freeing in the error path. Fixes: c77e22834ae9 ("NFSv4: Fix a potential sleep while atomic in nfs4_do_reclaim()") Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> --- fs/nfs/nfs4state.c | 1 + 1 file changed, 1 insertion(+)