All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyunsol Mun <muumthf@gmail.com>
To: linux-nfs@vger.kernel.org
Cc: Chuck Lever <cel@kernel.org>, Jeff Layton <jlayton@kernel.org>,
	NeilBrown <neil@brown.name>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	bobtobabz@gmail.com, Hyunsol Mun <muumthf@gmail.com>
Subject: [PATCH v3] nfsd: pin source client while using COPY_NOTIFY stateid
Date: Mon, 12 Oct 2026 19:04:35 +0900	[thread overview]
Message-ID: <20261012100435.144767-1-muumthf@gmail.com> (raw)
In-Reply-To: <5395aa50-bf30-492b-809d-fc882ea5ac57@app.fastmail.com>

find_cpntf_state() returns a referenced stateid after dropping the RPC
reference acquired by lookup_clientid() for its source client. A stateid
reference does not retain sc_client, so source-client expiry can free the
client while stateid validation or nfs4_put_stid() still uses it.

Move COPY_NOTIFY stateid processing into nfs4_preprocess_cpntf_stateid(),
called from the READ, READ_PLUS and SEEK paths. Keep the source
client's existing RPC reference through stateid validation, file lookup,
and nfs4_put_stid(), then release it with put_client_renew(). The reference
prevents client teardown while the stateid is checked and released. Only a
referenced nfsd_file escapes the helper for the subsequent I/O; neither the
source client nor its stateid escapes the helper.

An expired client can remain in the client ID table while
force_expire_client() waits for its RPC users. Reject it in the common
table lookup before renewing its lease or returning it to other callers.
lookup_clientid() holds nn->client_lock across this check and the RPC
reference acquisition, so it cannot pin a client after administrator
expiry has started draining its users.

Remove the COPY_NOTIFY fallback from nfs4_preprocess_stateid_op() and keep
its interface unchanged. Other callers no longer interpret COPY_NOTIFY
tokens as ordinary stateids. Share the existing generation, access-mode,
and filehandle checks between the two paths, and retain the existing file
permission checks for COPY_NOTIFY reads.

On the unmodified KASAN kernel at the base recorded below, five
timing-assisted trials entered the widened interval three times.
Source-client replacement freed the client during that interval twice,
and one trial reported a slab-use-after-free in nfs4_put_stid() through
nfs4_preprocess_stateid_op() and nfsd4_read(). A reliable trigger
without timing assistance was not demonstrated.

Timing assistance used a test-only kprobe to delay nfsd_permission()
after COPY_NOTIFY state lookup. The kprobe is not part of this patch;
it did not allocate, free, or modify NFSD objects.

With this patch applied to the same base, the full KASAN kernel built
with CONFIG_WERROR without a compiler diagnostic and booted successfully.

All 25 timing-assisted patched trials entered the widened interval. The
source client was not freed there, no KASAN report or kernel failure was
observed, and all token READs completed with NFS4_OK and 23 bytes after
client replacement returned NFS4ERR_DELAY. Twenty additional trials
without timing assistance also produced no KASAN report or kernel failure.
Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests passed.

Five patched READ_PLUS trials and five patched SEEK trials had three and
four timing-probe hits, respectively. The hit requests returned NFS4_OK;
the other races returned NFS4ERR_EXPIRED. Three administrator-expiry
trials overlapped a delayed token READ: the expire write waited for the
in-flight request, while a second token READ returned NFS4ERR_EXPIRED.
All three removed the source client without a KASAN report or kernel
failure.

Additional client-ID probes during administrator expiry returned
NFS4ERR_STALE_CLIENTID for CREATE_SESSION and DESTROY_CLIENTID. A separate
NFSv4.0 test returned the same status for a new SETCLIENTID_CONFIRM while
an earlier confirmation completed and the expire write waited for it.

A source reproducer, timing-module source, complete logs, and the kernel
configuration are available privately on request.

The vulnerability research and validation were conducted by members of
the Tobabz team as part of the Best of the Best 15th program.

Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Assisted-by: LLM
Signed-off-by: Hyunsol Mun <muumthf@gmail.com>
---
Changes in v3:
- Rebase onto the refreshed nfsd-testing tree without adding another
  nfsd4_get_client() helper or changing existing kref_get() callers.
- Move COPY_NOTIFY processing into a helper used by READ, READ_PLUS and
  SEEK, without extending the generic stateid API.
- Keep the source client's RPC reference until stateid release, and
  reject already-expired clients before renewal and reference acquisition.

 fs/nfsd/nfs4proc.c  |  9 +++++
 fs/nfsd/nfs4state.c | 87 +++++++++++++++++++++++++++++----------------
 fs/nfsd/state.h     |  4 +++
 3 files changed, 70 insertions(+), 30 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index eb3a3095ea9e..0848baad6842 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1148,6 +1148,11 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
 					&read->rd_stateid, RD_STATE,
 					&read->rd_nf, NULL);
+	if (status == nfserr_bad_stateid)
+		status = nfs4_preprocess_cpntf_stateid(rqstp, cstate,
+						       &cstate->current_fh,
+						       &read->rd_stateid,
+						       &read->rd_nf);
 
 	read->rd_rqstp = rqstp;
 	read->rd_fhp = &cstate->current_fh;
@@ -2621,6 +2626,10 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
 					    &seek->seek_stateid,
 					    RD_STATE, &nf, NULL);
+	if (status == nfserr_bad_stateid)
+		status = nfs4_preprocess_cpntf_stateid(rqstp, cstate,
+						       &cstate->current_fh,
+						       &seek->seek_stateid, &nf);
 	if (status)
 		return status;
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 920967dd6f84..26d2d56a51e5 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4070,6 +4070,8 @@ find_client_in_id_table(struct list_head *tbl, clientid_t *clid, bool sessions)
 		if (same_clid(&clp->cl_clientid, clid)) {
 			if ((bool)clp->cl_minorversion != sessions)
 				return NULL;
+			if (is_client_expired(clp))
+				return NULL;
 			renew_client_locked(clp);
 			return clp;
 		}
@@ -8528,14 +8530,53 @@ __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 	return 0;
 }
 
-static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
-			       struct nfs4_stid **stid)
+static __be32 nfs4_check_io_stateid(struct svc_fh *fhp, stateid_t *stateid,
+				    struct nfs4_stid *s, int flags, bool has_session)
 {
 	__be32 status;
+
+	status = nfsd4_stid_check_stateid_generation(stateid, s, has_session);
+	if (status)
+		return status;
+
+	switch (s->sc_type) {
+	case SC_TYPE_DELEG:
+		status = nfs4_check_delegmode(delegstateid(s), flags);
+		break;
+	case SC_TYPE_OPEN:
+	case SC_TYPE_LOCK:
+		status = nfs4_check_olstateid(openlockstateid(s), flags);
+		break;
+	}
+	if (status)
+		return status;
+	return nfs4_check_fh(fhp, s);
+}
+
+/**
+ * nfs4_preprocess_cpntf_stateid - prepare a COPY_NOTIFY stateid for read-side I/O
+ * @rqstp: incoming request from the destination server
+ * @cstate: current compound state
+ * @fhp: filehandle associated with the requested stateid
+ * @stateid: COPY_NOTIFY stateid provided by the destination server
+ * @nfp: returned nfsd_file reference
+ *
+ * Keep the source client's RPC reference until the stateid has been released.
+ * Only the file reference escapes to the caller, for READ, READ_PLUS or SEEK.
+ */
+__be32 nfs4_preprocess_cpntf_stateid(struct svc_rqst *rqstp,
+				     struct nfsd4_compound_state *cstate,
+				     struct svc_fh *fhp, stateid_t *stateid,
+				     struct nfsd_file **nfp)
+{
+	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 	struct nfs4_cpntf_state *cps = NULL;
 	struct nfs4_client *found;
+	struct nfs4_stid *stid;
+	__be32 status;
 
-	status = manage_cpntf_state(nn, st, NULL, &cps);
+	*nfp = NULL;
+	status = manage_cpntf_state(nn, stateid, NULL, &cps);
 	if (status)
 		return status;
 
@@ -8546,14 +8587,18 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 	if (!found)
 		goto out;
 
-	*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
-				     SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
-				     0);
-	if (*stid)
-		status = nfs_ok;
-	else
-		status = nfserr_bad_stateid;
+	status = nfserr_bad_stateid;
+	stid = find_stateid_by_type(found, &cps->cp_p_stateid,
+				    SC_TYPE_DELEG | SC_TYPE_OPEN | SC_TYPE_LOCK, 0);
+	if (!stid)
+		goto out_put_client;
 
+	status = nfs4_check_io_stateid(fhp, stateid, stid, RD_STATE,
+				       nfsd4_has_session(cstate));
+	if (!status)
+		status = nfs4_check_file(rqstp, fhp, stid, nfp, RD_STATE);
+	nfs4_put_stid(stid);
+out_put_client:
 	put_client_renew(found);
 out:
 	nfs4_put_cpntf_state(nn, cps);
@@ -8603,32 +8648,14 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
 	status = nfsd4_lookup_stateid(cstate, stateid,
 				SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
 				0, &s, nn);
-	if (status == nfserr_bad_stateid)
-		status = find_cpntf_state(nn, stateid, &s);
 	if (status)
 		return status;
-	status = nfsd4_stid_check_stateid_generation(stateid, s,
-			nfsd4_has_session(cstate));
-	if (status)
-		goto out;
-
-	switch (s->sc_type) {
-	case SC_TYPE_DELEG:
-		status = nfs4_check_delegmode(delegstateid(s), flags);
-		break;
-	case SC_TYPE_OPEN:
-	case SC_TYPE_LOCK:
-		status = nfs4_check_olstateid(openlockstateid(s), flags);
-		break;
-	}
-	if (status)
-		goto out;
-	status = nfs4_check_fh(fhp, s);
+	status = nfs4_check_io_stateid(fhp, stateid, s, flags,
+				       nfsd4_has_session(cstate));
 
 done:
 	if (status == nfs_ok && nfp)
 		status = nfs4_check_file(rqstp, fhp, s, nfp, flags);
-out:
 	if (s) {
 		if (!status && cstid)
 			*cstid = s;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 910f4eac71d0..79dfa47b82db 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -914,6 +914,10 @@ extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, struct svc_fh *fhp,
 		stateid_t *stateid, int flags, struct nfsd_file **filp,
 		struct nfs4_stid **cstid);
+__be32 nfs4_preprocess_cpntf_stateid(struct svc_rqst *rqstp,
+				     struct nfsd4_compound_state *cstate,
+				     struct svc_fh *fhp, stateid_t *stateid,
+				     struct nfsd_file **nfp);
 __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 			    stateid_t *stateid, unsigned short typemask,
 			    unsigned short statusmask,

base-commit: d918c664efd5ec6a7309e98f567fb018bcde0e11

      reply	other threads:[~2026-10-12 10:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30  5:39 [PATCH v2] nfsd: pin source client while using COPY_NOTIFY stateid Hyunsol Mun
2026-10-02  7:20 ` Jeff Layton
2026-10-02 15:55   ` Chuck Lever
2026-10-12 10:04     ` Hyunsol Mun [this message]

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=20261012100435.144767-1-muumthf@gmail.com \
    --to=muumthf@gmail.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=bobtobabz@gmail.com \
    --cc=cel@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.