[PATCH] nfsd: pin client reference across async copy reaper cleanup

[PATCH] nfsd: pin client reference across async copy reaper cleanup

[Tristan Madani]

nfsd4_async_copy_reaper() detaches completed async copies from the
client list and later calls cleanup_async_copy(), which eventually
drops the stid reference via nfs4_put_stid().  That path accesses
stid->sc_client unconditionally, but the client is not pinned during
the cleanup phase.

A concurrent expire_client() or force_expire_client() can free the
client between the nn->client_lock drop and the cleanup loop, leaving
nfs4_put_stid() with a stale sc_client pointer.

This is the same missed pattern that Chuck Lever addressed in seven
other nfsd4_laundromat() paths with cl_rpc_users pinning (v7.3-rc2).
The async copy reaper was not covered by that series.

Fix this by:
  1. Skipping expired clients (cl_time == 0) during iteration.
  2. Taking a cl_rpc_users reference before moving the copy to the
     reaplist.
  3. Dropping the reference with put_client_no_renew() after
     cleanup_async_copy().

Export put_client_no_renew() so it is accessible from nfs4proc.c.

Fixes: ac0514f4d198 ("NFSD: Add a laundromat reaper for async copy state")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 fs/nfsd/nfs4proc.c  | 5 +++++
 fs/nfsd/nfs4state.c | 2 +-
 fs/nfsd/state.h     | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 50c07561e31f3..1f36afceb1fe1 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1501,6 +1501,9 @@ void nfsd4_async_copy_reaper(struct nfsd_net *nn)
 			if (test_bit(NFSD4_COPY_F_OFFLOAD_DONE,
 				     &copy->cp_copy.cp_flags)) {
 				if (!--copy->cp_ttl) {
+					if (clp->cl_time == 0)
+						continue;
+					atomic_inc(&clp->cl_rpc_users);
 					list_del_init(&copy->copies);
 					list_add(&copy->copies, &reaplist);
 				}
@@ -1513,8 +1516,10 @@ void nfsd4_async_copy_reaper(struct nfsd_net *nn)
 	while (!list_empty(&reaplist)) {
 		copy = list_first_entry(&reaplist, struct nfsd4_async_copy,
 					copies);
+		clp = copy->cp_stid.sc_client;
 		list_del_init(&copy->copies);
 		cleanup_async_copy(copy);
+		put_client_no_renew(clp);
 	}
 }
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9c4adf3110aea..10e864ca7407a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -252,7 +252,7 @@ static void put_client_no_renew_locked(struct nfs4_client *clp)
 		__put_client_locked(clp, false);
 }
 
-static void put_client_no_renew(struct nfs4_client *clp)
+void put_client_no_renew(struct nfs4_client *clp)
 {
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 2d00a411c6634..1d3a7e35bb9ff 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -918,6 +918,7 @@ extern void nfsd4_shutdown_callback(struct nfs4_client *);
 extern void nfsd4_shutdown_copy(struct nfs4_client *clp);
 void nfsd4_put_client(struct nfs4_client *clp);
 void nfsd4_async_copy_reaper(struct nfsd_net *nn);
+void put_client_no_renew(struct nfs4_client *clp);
 bool nfsd4_has_active_async_copies(struct nfs4_client *clp);
 void nfsd_update_cmtime_attr(struct file *f, unsigned int flags);
 extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(struct xdr_netobj name,
-- 
2.47.3

Re: [PATCH] nfsd: pin client reference across async copy reaper cleanup

[Jeff Layton]

On Mon, 2026-10-26 at 23:01 +0000, Tristan Madani wrote:
> nfsd4_async_copy_reaper() detaches completed async copies from the
> client list and later calls cleanup_async_copy(), which eventually
> drops the stid reference via nfs4_put_stid().  That path accesses
> stid->sc_client unconditionally, but the client is not pinned during
> the cleanup phase.
> 
> A concurrent expire_client() or force_expire_client() can free the
> client between the nn->client_lock drop and the cleanup loop, leaving
> nfs4_put_stid() with a stale sc_client pointer.
> 
> This is the same missed pattern that Chuck Lever addressed in seven
> other nfsd4_laundromat() paths with cl_rpc_users pinning (v7.3-rc2).
> The async copy reaper was not covered by that series.
> 
> Fix this by:
>   1. Skipping expired clients (cl_time == 0) during iteration.
>   2. Taking a cl_rpc_users reference before moving the copy to the
>      reaplist.
>   3. Dropping the reference with put_client_no_renew() after
>      cleanup_async_copy().
> 
> Export put_client_no_renew() so it is accessible from nfs4proc.c.
> 
> Fixes: ac0514f4d198 ("NFSD: Add a laundromat reaper for async copy state")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>  fs/nfsd/nfs4proc.c  | 5 +++++
>  fs/nfsd/nfs4state.c | 2 +-
>  fs/nfsd/state.h     | 1 +
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 50c07561e31f3..1f36afceb1fe1 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1501,6 +1501,9 @@ void nfsd4_async_copy_reaper(struct nfsd_net *nn)
>  			if (test_bit(NFSD4_COPY_F_OFFLOAD_DONE,
>  				     &copy->cp_copy.cp_flags)) {
>  				if (!--copy->cp_ttl) {
> +					if (clp->cl_time == 0)
> +						continue;
> +					atomic_inc(&clp->cl_rpc_users);
>  					list_del_init(&copy->copies);
>  					list_add(&copy->copies, &reaplist);
>  				}
> @@ -1513,8 +1516,10 @@ void nfsd4_async_copy_reaper(struct nfsd_net *nn)
>  	while (!list_empty(&reaplist)) {
>  		copy = list_first_entry(&reaplist, struct nfsd4_async_copy,
>  					copies);
> +		clp = copy->cp_stid.sc_client;
>  		list_del_init(&copy->copies);
>  		cleanup_async_copy(copy);
> +		put_client_no_renew(clp);
>  	}
>  }
>  
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 9c4adf3110aea..10e864ca7407a 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -252,7 +252,7 @@ static void put_client_no_renew_locked(struct nfs4_client *clp)
>  		__put_client_locked(clp, false);
>  }
>  
> -static void put_client_no_renew(struct nfs4_client *clp)
> +void put_client_no_renew(struct nfs4_client *clp)
>  {
>  	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
>  
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 2d00a411c6634..1d3a7e35bb9ff 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -918,6 +918,7 @@ extern void nfsd4_shutdown_callback(struct nfs4_client *);
>  extern void nfsd4_shutdown_copy(struct nfs4_client *clp);
>  void nfsd4_put_client(struct nfs4_client *clp);
>  void nfsd4_async_copy_reaper(struct nfsd_net *nn);
> +void put_client_no_renew(struct nfs4_client *clp);
>  bool nfsd4_has_active_async_copies(struct nfs4_client *clp);
>  void nfsd_update_cmtime_attr(struct file *f, unsigned int flags);
>  extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(struct xdr_netobj name,

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH] nfsd: pin client reference across async copy reaper cleanup

[Chuck Lever]

On 10/26/26 7:01 PM, Tristan Madani wrote:
> nfsd4_async_copy_reaper() detaches completed async copies from the
> client list and later calls cleanup_async_copy(), which eventually
> drops the stid reference via nfs4_put_stid().  That path accesses
> stid->sc_client unconditionally, but the client is not pinned during
> the cleanup phase.

Thanks for the report. The same bug is already fixed here:

  0941ff1cda56 ("nfsd: hold client reference while reaping async copies")
  https://lore.kernel.org/linux-nfs/20260928083050.642664-1-bint4b13@gmail.com/

For future NFSD submissions, please base on the nfsd-testing
branch at

  https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git

per Documentation/filesystems/nfs/nfsd-maintainer-entry-profile.rst.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)