[PATCH 6.12] NFSD: net ref data still needs to be freed even if net hasn't startup

[PATCH 6.12] NFSD: net ref data still needs to be freed even if net hasn't startup

[Jay Wang]

From: Edward Adam Davis <eadavis@qq.com>

When the NFSD instance doesn't to startup, the net ref data memory is
not properly reclaimed, which triggers the memory leak issue reported
by syzbot [1].

To avoid the problem reported in [1], the net ref data memory reclamation
action is moved outside of nfsd_net_up when the net is shutdown.

[1]
unreferenced object 0xffff88812a39dfc0 (size 64):
  backtrace (crc a2262fc6):
    percpu_ref_init+0x94/0x1e0 lib/percpu-refcount.c:76
    nfsd_create_serv+0xbe/0x260 fs/nfsd/nfssvc.c:605
    nfsd_nl_listener_set_doit+0x62/0xb00 fs/nfsd/nfsctl.c:1882
    genl_family_rcv_msg_doit+0x11e/0x190 net/netlink/genetlink.c:1115
    genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
    genl_rcv_msg+0x2fd/0x440 net/netlink/genetlink.c:1210

BUG: memory leak

Reported-by: syzbot+6ee3b889bdeada0a6226@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6ee3b889bdeada0a6226
Fixes: 39972494e318 ("nfsd: update percpu_ref to manage references on nfsd_net")
Cc: stable@vger.kernel.org
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Jay Wang <wanjay@amazon.com>
---
 fs/nfsd/nfssvc.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index cc185c00e309..88c15b49e4bd 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -434,26 +434,26 @@ static void nfsd_shutdown_net(struct net *net)
 {
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
 
-	if (!nn->nfsd_net_up)
-		return;
-
-	percpu_ref_kill_and_confirm(&nn->nfsd_net_ref, nfsd_net_done);
-	wait_for_completion(&nn->nfsd_net_confirm_done);
-
-	nfsd_export_flush(net);
-	nfs4_state_shutdown_net(net);
-	nfsd_reply_cache_shutdown(nn);
-	nfsd_file_cache_shutdown_net(net);
-	if (nn->lockd_up) {
-		lockd_down(net);
-		nn->lockd_up = false;
+	if (nn->nfsd_net_up) {
+		percpu_ref_kill_and_confirm(&nn->nfsd_net_ref, nfsd_net_done);
+		wait_for_completion(&nn->nfsd_net_confirm_done);
+
+		nfsd_export_flush(net);
+		nfs4_state_shutdown_net(net);
+		nfsd_reply_cache_shutdown(nn);
+		nfsd_file_cache_shutdown_net(net);
+		if (nn->lockd_up) {
+			lockd_down(net);
+			nn->lockd_up = false;
+		}
+		wait_for_completion(&nn->nfsd_net_free_done);
 	}
 
-	wait_for_completion(&nn->nfsd_net_free_done);
 	percpu_ref_exit(&nn->nfsd_net_ref);
 
+	if (nn->nfsd_net_up)
+		nfsd_shutdown_generic();
 	nn->nfsd_net_up = false;
-	nfsd_shutdown_generic();
 }
 
 static DEFINE_SPINLOCK(nfsd_notifier_lock);
-- 
2.47.3

Re: [PATCH 6.12] NFSD: net ref data still needs to be freed even if net hasn't startup

[Harshit Mogalapalli]

Hi Jay,

On 13/01/26 00:28, Jay Wang wrote:
> From: Edward Adam Davis <eadavis@qq.com>
> 
> When the NFSD instance doesn't to startup, the net ref data memory is
> not properly reclaimed, which triggers the memory leak issue reported
> by syzbot [1].
> 
> To avoid the problem reported in [1], the net ref data memory reclamation
> action is moved outside of nfsd_net_up when the net is shutdown.
> 
> [1]
> unreferenced object 0xffff88812a39dfc0 (size 64):
>    backtrace (crc a2262fc6):
>      percpu_ref_init+0x94/0x1e0 lib/percpu-refcount.c:76
>      nfsd_create_serv+0xbe/0x260 fs/nfsd/nfssvc.c:605
>      nfsd_nl_listener_set_doit+0x62/0xb00 fs/nfsd/nfsctl.c:1882
>      genl_family_rcv_msg_doit+0x11e/0x190 net/netlink/genetlink.c:1115
>      genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
>      genl_rcv_msg+0x2fd/0x440 net/netlink/genetlink.c:1210
> 
> BUG: memory leak
> 
> Reported-by: syzbot+6ee3b889bdeada0a6226@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6ee3b889bdeada0a6226
> Fixes: 39972494e318 ("nfsd: update percpu_ref to manage references on nfsd_net")
> Cc: stable@vger.kernel.org
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> Signed-off-by: Jay Wang <wanjay@amazon.com>

You need to mention the upstream commit while cherry-picking/backporting 
a commit to stable/lts kernels.

it could either be of format:

[Upstream commit 0b88bfa42e5468baff71909c2f324a495318532b]

or

commit 0b88bfa42e5468baff71909c2f324a495318532b upstream.

on the first line of the backport. [1]


Looks like this is this commit in mainline : v6.19-rc5 - 0b88bfa42e54 
NFSD: net ref data still needs to be freed even if net hasn't startup

Also, while backporting to a long-term-stable kernel also please ensure 
the backport is present in all higher/newer stable/LTS version, so we 
don't let our stable tree consumers run into regressions when they 
update to newer releases. So in this case, 6.18.y also needs to be patched.

[1] https://www.kernel.org/doc/html/v6.18/process/stable-kernel-rules.html

Thanks,
Harshit
> ---
>   fs/nfsd/nfssvc.c | 30 +++++++++++++++---------------
>   1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index cc185c00e309..88c15b49e4bd 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -434,26 +434,26 @@ static void nfsd_shutdown_net(struct net *net)
>   {
>   	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>   
> -	if (!nn->nfsd_net_up)
> -		return;
> -
> -	percpu_ref_kill_and_confirm(&nn->nfsd_net_ref, nfsd_net_done);
> -	wait_for_completion(&nn->nfsd_net_confirm_done);
> -
> -	nfsd_export_flush(net);
> -	nfs4_state_shutdown_net(net);
> -	nfsd_reply_cache_shutdown(nn);
> -	nfsd_file_cache_shutdown_net(net);
> -	if (nn->lockd_up) {
> -		lockd_down(net);
> -		nn->lockd_up = false;
> +	if (nn->nfsd_net_up) {
> +		percpu_ref_kill_and_confirm(&nn->nfsd_net_ref, nfsd_net_done);
> +		wait_for_completion(&nn->nfsd_net_confirm_done);
> +
> +		nfsd_export_flush(net);
> +		nfs4_state_shutdown_net(net);
> +		nfsd_reply_cache_shutdown(nn);
> +		nfsd_file_cache_shutdown_net(net);
> +		if (nn->lockd_up) {
> +			lockd_down(net);
> +			nn->lockd_up = false;
> +		}
> +		wait_for_completion(&nn->nfsd_net_free_done);
>   	}
>   
> -	wait_for_completion(&nn->nfsd_net_free_done);
>   	percpu_ref_exit(&nn->nfsd_net_ref);
>   
> +	if (nn->nfsd_net_up)
> +		nfsd_shutdown_generic();
>   	nn->nfsd_net_up = false;
> -	nfsd_shutdown_generic();
>   }
>   
>   static DEFINE_SPINLOCK(nfsd_notifier_lock);