linux-kernel - Re: [PATCH] nfs: fix memory leak in error path of nfs4_do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8ee4ce7c4eb56ec80365492407b76ee3dc4b6347.camel@kernel.org>
Date: Wed, 04 Sep 2024 10:38:45 -0400
From: Trond Myklebust <trondmy@...nel.org>
To: Li Lingfeng <lilingfeng3@...wei.com>, anna@...nel.org
Cc: jlayton@...nel.org, linux-nfs@...r.kernel.org,
 linux-kernel@...r.kernel.org,  yukuai1@...weicloud.com, houtao1@...wei.com,
 yi.zhang@...wei.com,  yangerkun@...wei.com, lilingfeng@...weicloud.com
Subject: Re: [PATCH] nfs: fix memory leak in error path of nfs4_do_reclaim

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@...wei.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.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@...merspace.com