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,
> ©->cp_copy.cp_flags)) {
> if (!--copy->cp_ttl) {
> + if (clp->cl_time == 0)
> + continue;
> + atomic_inc(&clp->cl_rpc_users);
> list_del_init(©->copies);
> list_add(©->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(©->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>
next prev parent 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®