mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Tristan Madani <tristmd@gmail.com>, Chuck Lever <cel@kernel.org>
Cc: NeilBrown <neil@brown.name>,
	Olga Kornievskaia <okorniev@redhat.com>,
	 Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	linux-nfs@vger.kernel.org, 	linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, 	tristan@talencesecurity.com
Subject: Re: [PATCH] nfsd: pin client reference across async copy reaper cleanup
Date: Tue, 27 Oct 2026 06:58:30 -0400	[thread overview]
Message-ID: <715d2efa3fd7abe8ce9a42f365bac9f88912b6ad.camel@kernel.org> (raw)
In-Reply-To: <20261026230151.2474608-1-tristan@talencesecurity.com>

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>

  reply	other threads:[~2026-10-27 10:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-26 23:01 Tristan Madani
2026-10-27 10:58 ` Jeff Layton [this message]
2026-10-27 13:38 ` Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=715d2efa3fd7abe8ce9a42f365bac9f88912b6ad.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=Dai.Ngo@oracle.com \
    --cc=cel@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=tom@talpey.com \
    --cc=tristan@talencesecurity.com \
    --cc=tristmd@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®