* [PATCH v2] nfsd: pin source client while using COPY_NOTIFY stateid
@ 2026-09-30 5:39 Hyunsol Mun
2026-10-02 7:20 ` Jeff Layton
0 siblings, 1 reply; 4+ messages in thread
From: Hyunsol Mun @ 2026-09-30 5:39 UTC (permalink / raw)
To: linux-nfs
Cc: Chuck Lever, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
Tom Talpey, bobtobabz, Hyunsol Mun
A COPY_NOTIFY token can resolve to a stateid owned by the source client.
find_cpntf_state() drops its source-client reference before returning
that stateid, so concurrent source-client expiry can free sc_client
while stateid validation or the caller still uses the stateid.
Add a local nfsd4_get_client() helper as the counterpart to
nfsd4_put_client(). Return a paired client reference from
nfs4_preprocess_stateid_op() when lookup crosses through COPY_NOTIFY
state. Keep it until the caller releases the returned stateid; internal
and error paths release it locally.
An unprivileged NFSv4.2 client with access to the exported file can execute
the OPEN, COPY_NOTIFY, source-client replacement, and token READ sequence.
Four hundred natural trials reached the protocol path without reproducing
the report, so a reliable timing-assistance-free trigger was not
demonstrated.
For deterministic validation, a test-only kprobe delayed nfsd_permission()
after COPY_NOTIFY state lookup. The module only widened the race window; it
did not allocate, free, or modify an NFSD object. Validation used the
nfsd-testing base recorded below. On the unmodified KASAN kernel, five
trials registered four delayed calls and reproduced the source-client
use-after-free in nfs4_put_stid(). With this patch, five trials registered
five delayed calls and produced no KASAN report or kernel failure. Three
token READs returned NFS4_OK with 23 bytes; two returned NFS4ERR_EXPIRED as
client replacement won the race.
The full KASAN kernel built with CONFIG_WERROR without a compiler
diagnostic. Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests passed.
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 v2:
- Describe current timing-assisted baseline and patched results.
- Add full-build and NFSv4.2/NFSv3 smoke-test results.
- Keep nfsd4_get_client() local to nfs4state.c.
fs/nfsd/nfs4proc.c | 31 +++++++++++++++++++++----------
fs/nfsd/nfs4state.c | 34 +++++++++++++++++++++++++++-------
fs/nfsd/state.h | 2 +-
3 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 7df60abfb..d5b5f59f1 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1144,7 +1144,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
/* check stateid */
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
&read->rd_stateid, RD_STATE,
- &read->rd_nf, NULL);
+ &read->rd_nf, NULL, NULL);
read->rd_rqstp = rqstp;
read->rd_fhp = &cstate->current_fh;
@@ -1342,6 +1342,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
.na_dpacl = posix_acl_dup(setattr->sa_dpacl),
};
bool save_no_wcc, deleg_attrs;
+ struct nfs4_client *stid_clp = NULL;
struct nfs4_stid *st = NULL;
struct inode *inode;
__be32 status = nfs_ok;
@@ -1358,7 +1359,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
status = nfs4_preprocess_stateid_op(rqstp, cstate,
&cstate->current_fh, &setattr->sa_stateid,
- flags, NULL, &st);
+ flags, NULL, &st, &stid_clp);
if (status)
goto out_err;
}
@@ -1375,8 +1376,11 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
}
}
}
- if (st)
+ if (st) {
nfs4_put_stid(st);
+ if (stid_clp)
+ nfsd4_put_client(stid_clp);
+ }
if (status)
goto out_err;
@@ -1439,6 +1443,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_write *write = &u->write;
stateid_t *stateid = &write->wr_stateid;
struct nfs4_stid *stid = NULL;
+ struct nfs4_client *stid_clp = NULL;
struct nfsd_file *nf = NULL;
__be32 status = nfs_ok;
unsigned long cnt;
@@ -1451,13 +1456,16 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
trace_nfsd_write_start(rqstp, &cstate->current_fh,
write->wr_offset, cnt);
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
- stateid, WR_STATE, &nf, &stid);
+ stateid, WR_STATE, &nf, &stid,
+ &stid_clp);
if (status)
return status;
if (stid) {
nfsd4_file_mark_deleg_written(stid->sc_file);
nfs4_put_stid(stid);
+ if (stid_clp)
+ nfsd4_put_client(stid_clp);
}
write->wr_how_written = write->wr_stable_how;
@@ -1484,12 +1492,12 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return nfserr_nofilehandle;
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
- src_stateid, RD_STATE, src, NULL);
+ src_stateid, RD_STATE, src, NULL, NULL);
if (status)
goto out;
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
- dst_stateid, WR_STATE, dst, NULL);
+ dst_stateid, WR_STATE, dst, NULL, NULL);
if (status)
goto out_put_src;
@@ -1954,7 +1962,7 @@ nfsd4_setup_inter_ssc(struct svc_rqst *rqstp,
/* Verify the destination stateid and set dst struct file*/
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
©->cp_dst_stateid,
- WR_STATE, ©->nf_dst, NULL);
+ WR_STATE, ©->nf_dst, NULL, NULL);
if (status)
goto out;
@@ -2498,12 +2506,13 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_copy_notify *cn = &u->copy_notify;
__be32 status;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+ struct nfs4_client *stid_clp = NULL;
struct nfs4_stid *stid = NULL;
struct nfs4_cpntf_state *cps;
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
&cn->cpn_src_stateid, RD_STATE, NULL,
- &stid);
+ &stid, &stid_clp);
if (status)
return status;
if (!stid)
@@ -2536,6 +2545,8 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
nfs4_put_cpntf_state(nn, cps);
out:
nfs4_put_stid(stid);
+ if (stid_clp)
+ nfsd4_put_client(stid_clp);
return status;
}
@@ -2548,7 +2559,7 @@ nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
&fallocate->falloc_stateid,
- WR_STATE, &nf, NULL);
+ WR_STATE, &nf, NULL, NULL);
if (status != nfs_ok)
return status;
@@ -2612,7 +2623,7 @@ 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);
+ RD_STATE, &nf, NULL, NULL);
if (status)
return status;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index bbc16dd22..43906f87e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2805,6 +2805,15 @@ static void __free_client(struct kref *k)
kmem_cache_free(client_slab, clp);
}
+/**
+ * nfsd4_get_client - acquire a reference on an nfs4_client
+ * @clp: the client to be acquired
+ */
+static void nfsd4_get_client(struct nfs4_client *clp)
+{
+ kref_get(&clp->cl_nfsdfs.cl_ref);
+}
+
/**
* nfsd4_put_client - release a reference on an nfs4_client
* @clp: the client to be released
@@ -8504,7 +8513,8 @@ __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
}
static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
- struct nfs4_stid **stid)
+ struct nfs4_stid **stid,
+ struct nfs4_client **stid_clp)
{
__be32 status;
struct nfs4_cpntf_state *cps = NULL;
@@ -8524,10 +8534,13 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
0);
- if (*stid)
+ if (*stid) {
+ nfsd4_get_client(found);
+ *stid_clp = found;
status = nfs_ok;
- else
+ } else {
status = nfserr_bad_stateid;
+ }
put_client_renew(found);
out:
@@ -8551,6 +8564,7 @@ void nfs4_put_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
* @flags: flags describing type of operation to be done
* @nfp: optional nfsd_file return pointer (may be NULL)
* @cstid: optional returned nfs4_stid pointer (may be NULL)
+ * @cstid_clp: client reference paired with @cstid (required with @cstid)
*
* Given info from the client, look up a nfs4_stid for the operation. On
* success, it returns a reference to the nfs4_stid and/or the nfsd_file
@@ -8560,10 +8574,11 @@ __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 **nfp,
- struct nfs4_stid **cstid)
+ struct nfs4_stid **cstid, struct nfs4_client **cstid_clp)
{
struct net *net = SVC_NET(rqstp);
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+ struct nfs4_client *stid_clp = NULL;
struct nfs4_stid *s = NULL;
__be32 status;
@@ -8579,7 +8594,7 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
0, &s, nn);
if (status == nfserr_bad_stateid)
- status = find_cpntf_state(nn, stateid, &s);
+ status = find_cpntf_state(nn, stateid, &s, &stid_clp);
if (status)
return status;
status = nfsd4_stid_check_stateid_generation(stateid, s,
@@ -8605,11 +8620,16 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
status = nfs4_check_file(rqstp, fhp, s, nfp, flags);
out:
if (s) {
- if (!status && cstid)
+ if (!status && cstid) {
*cstid = s;
- else
+ *cstid_clp = stid_clp;
+ stid_clp = NULL;
+ } else {
nfs4_put_stid(s);
+ }
}
+ if (stid_clp)
+ nfsd4_put_client(stid_clp);
return status;
}
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 1a0da82cb..7910dabb4 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -911,7 +911,7 @@ struct nfsd4_async_copy;
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);
+ struct nfs4_stid **cstid, struct nfs4_client **cstid_clp);
__be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
stateid_t *stateid, unsigned short typemask,
unsigned short statusmask,
base-commit: 32eb1a60b456980761cf7a9cee8f907fdc08afb8
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] nfsd: pin source client while using COPY_NOTIFY stateid
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
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2026-10-02 7:20 UTC (permalink / raw)
To: Hyunsol Mun, linux-nfs
Cc: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
bobtobabz
On Wed, 2026-09-30 at 14:39 +0900, Hyunsol Mun wrote:
> A COPY_NOTIFY token can resolve to a stateid owned by the source client.
> find_cpntf_state() drops its source-client reference before returning
> that stateid, so concurrent source-client expiry can free sc_client
> while stateid validation or the caller still uses the stateid.
>
> Add a local nfsd4_get_client() helper as the counterpart to
> nfsd4_put_client(). Return a paired client reference from
> nfs4_preprocess_stateid_op() when lookup crosses through COPY_NOTIFY
> state. Keep it until the caller releases the returned stateid; internal
> and error paths release it locally.
>
> An unprivileged NFSv4.2 client with access to the exported file can execute
> the OPEN, COPY_NOTIFY, source-client replacement, and token READ sequence.
> Four hundred natural trials reached the protocol path without reproducing
> the report, so a reliable timing-assistance-free trigger was not
> demonstrated.
>
> For deterministic validation, a test-only kprobe delayed nfsd_permission()
> after COPY_NOTIFY state lookup. The module only widened the race window; it
> did not allocate, free, or modify an NFSD object. Validation used the
> nfsd-testing base recorded below. On the unmodified KASAN kernel, five
> trials registered four delayed calls and reproduced the source-client
> use-after-free in nfs4_put_stid(). With this patch, five trials registered
> five delayed calls and produced no KASAN report or kernel failure. Three
> token READs returned NFS4_OK with 23 bytes; two returned NFS4ERR_EXPIRED as
> client replacement won the race.
>
> The full KASAN kernel built with CONFIG_WERROR without a compiler
> diagnostic. Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests passed.
> 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 v2:
> - Describe current timing-assisted baseline and patched results.
> - Add full-build and NFSv4.2/NFSv3 smoke-test results.
> - Keep nfsd4_get_client() local to nfs4state.c.
>
> fs/nfsd/nfs4proc.c | 31 +++++++++++++++++++++----------
> fs/nfsd/nfs4state.c | 34 +++++++++++++++++++++++++++-------
> fs/nfsd/state.h | 2 +-
> 3 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 7df60abfb..d5b5f59f1 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1144,7 +1144,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> /* check stateid */
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> &read->rd_stateid, RD_STATE,
> - &read->rd_nf, NULL);
> + &read->rd_nf, NULL, NULL);
>
> read->rd_rqstp = rqstp;
> read->rd_fhp = &cstate->current_fh;
> @@ -1342,6 +1342,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> .na_dpacl = posix_acl_dup(setattr->sa_dpacl),
> };
> bool save_no_wcc, deleg_attrs;
> + struct nfs4_client *stid_clp = NULL;
> struct nfs4_stid *st = NULL;
> struct inode *inode;
> __be32 status = nfs_ok;
> @@ -1358,7 +1359,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>
> status = nfs4_preprocess_stateid_op(rqstp, cstate,
> &cstate->current_fh, &setattr->sa_stateid,
> - flags, NULL, &st);
> + flags, NULL, &st, &stid_clp);
> if (status)
> goto out_err;
> }
> @@ -1375,8 +1376,11 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> }
> }
> }
> - if (st)
> + if (st) {
> nfs4_put_stid(st);
> + if (stid_clp)
> + nfsd4_put_client(stid_clp);
> + }
> if (status)
> goto out_err;
>
> @@ -1439,6 +1443,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> struct nfsd4_write *write = &u->write;
> stateid_t *stateid = &write->wr_stateid;
> struct nfs4_stid *stid = NULL;
> + struct nfs4_client *stid_clp = NULL;
> struct nfsd_file *nf = NULL;
> __be32 status = nfs_ok;
> unsigned long cnt;
> @@ -1451,13 +1456,16 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> trace_nfsd_write_start(rqstp, &cstate->current_fh,
> write->wr_offset, cnt);
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> - stateid, WR_STATE, &nf, &stid);
> + stateid, WR_STATE, &nf, &stid,
> + &stid_clp);
> if (status)
> return status;
>
> if (stid) {
> nfsd4_file_mark_deleg_written(stid->sc_file);
> nfs4_put_stid(stid);
> + if (stid_clp)
> + nfsd4_put_client(stid_clp);
> }
>
> write->wr_how_written = write->wr_stable_how;
> @@ -1484,12 +1492,12 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> return nfserr_nofilehandle;
>
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
> - src_stateid, RD_STATE, src, NULL);
> + src_stateid, RD_STATE, src, NULL, NULL);
> if (status)
> goto out;
>
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> - dst_stateid, WR_STATE, dst, NULL);
> + dst_stateid, WR_STATE, dst, NULL, NULL);
> if (status)
> goto out_put_src;
>
> @@ -1954,7 +1962,7 @@ nfsd4_setup_inter_ssc(struct svc_rqst *rqstp,
> /* Verify the destination stateid and set dst struct file*/
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> ©->cp_dst_stateid,
> - WR_STATE, ©->nf_dst, NULL);
> + WR_STATE, ©->nf_dst, NULL, NULL);
> if (status)
> goto out;
>
> @@ -2498,12 +2506,13 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> struct nfsd4_copy_notify *cn = &u->copy_notify;
> __be32 status;
> struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> + struct nfs4_client *stid_clp = NULL;
> struct nfs4_stid *stid = NULL;
> struct nfs4_cpntf_state *cps;
>
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> &cn->cpn_src_stateid, RD_STATE, NULL,
> - &stid);
> + &stid, &stid_clp);
> if (status)
> return status;
> if (!stid)
> @@ -2536,6 +2545,8 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> nfs4_put_cpntf_state(nn, cps);
> out:
> nfs4_put_stid(stid);
> + if (stid_clp)
> + nfsd4_put_client(stid_clp);
> return status;
> }
>
> @@ -2548,7 +2559,7 @@ nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> &fallocate->falloc_stateid,
> - WR_STATE, &nf, NULL);
> + WR_STATE, &nf, NULL, NULL);
> if (status != nfs_ok)
> return status;
>
> @@ -2612,7 +2623,7 @@ 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);
> + RD_STATE, &nf, NULL, NULL);
> if (status)
> return status;
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index bbc16dd22..43906f87e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2805,6 +2805,15 @@ static void __free_client(struct kref *k)
> kmem_cache_free(client_slab, clp);
> }
>
> +/**
> + * nfsd4_get_client - acquire a reference on an nfs4_client
> + * @clp: the client to be acquired
> + */
> +static void nfsd4_get_client(struct nfs4_client *clp)
> +{
> + kref_get(&clp->cl_nfsdfs.cl_ref);
> +}
> +
If you're going to add this helper, then you should convert all of the
other places that call kref_get() on this object to use it, ideally in
a separate patch.
> /**
> * nfsd4_put_client - release a reference on an nfs4_client
> * @clp: the client to be released
> @@ -8504,7 +8513,8 @@ __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> }
>
> static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> - struct nfs4_stid **stid)
> + struct nfs4_stid **stid,
> + struct nfs4_client **stid_clp)
> {
> __be32 status;
> struct nfs4_cpntf_state *cps = NULL;
> @@ -8524,10 +8534,13 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> *stid = find_stateid_by_type(found, &cps->cp_p_stateid,
> SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
> 0);
> - if (*stid)
> + if (*stid) {
> + nfsd4_get_client(found);
> + *stid_clp = found;
> status = nfs_ok;
> - else
> + } else {
> status = nfserr_bad_stateid;
> + }
>
> put_client_renew(found);
> out:
> @@ -8551,6 +8564,7 @@ void nfs4_put_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
> * @flags: flags describing type of operation to be done
> * @nfp: optional nfsd_file return pointer (may be NULL)
> * @cstid: optional returned nfs4_stid pointer (may be NULL)
> + * @cstid_clp: client reference paired with @cstid (required with @cstid)
> *
> * Given info from the client, look up a nfs4_stid for the operation. On
> * success, it returns a reference to the nfs4_stid and/or the nfsd_file
> @@ -8560,10 +8574,11 @@ __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 **nfp,
> - struct nfs4_stid **cstid)
> + struct nfs4_stid **cstid, struct nfs4_client **cstid_clp)
The bug seems possible (esp given the fault-injected reproducer), but
this function is really devolving into a big mess. It would be very
nice to move the COPY-specific code into a dedicated helper that is
only called from the COPY-related codepaths. Consider reorganizing this
code along those lines.
> {
> struct net *net = SVC_NET(rqstp);
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> + struct nfs4_client *stid_clp = NULL;
> struct nfs4_stid *s = NULL;
> __be32 status;
>
> @@ -8579,7 +8594,7 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
> SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
> 0, &s, nn);
> if (status == nfserr_bad_stateid)
> - status = find_cpntf_state(nn, stateid, &s);
> + status = find_cpntf_state(nn, stateid, &s, &stid_clp);
> if (status)
> return status;
> status = nfsd4_stid_check_stateid_generation(stateid, s,
> @@ -8605,11 +8620,16 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
> status = nfs4_check_file(rqstp, fhp, s, nfp, flags);
> out:
> if (s) {
> - if (!status && cstid)
> + if (!status && cstid) {
> *cstid = s;
> - else
> + *cstid_clp = stid_clp;
> + stid_clp = NULL;
> + } else {
> nfs4_put_stid(s);
> + }
> }
> + if (stid_clp)
> + nfsd4_put_client(stid_clp);
> return status;
> }
>
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 1a0da82cb..7910dabb4 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -911,7 +911,7 @@ struct nfsd4_async_copy;
> 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);
> + struct nfs4_stid **cstid, struct nfs4_client **cstid_clp);
> __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> stateid_t *stateid, unsigned short typemask,
> unsigned short statusmask,
>
> base-commit: 32eb1a60b456980761cf7a9cee8f907fdc08afb8
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] nfsd: pin source client while using COPY_NOTIFY stateid
2026-10-02 7:20 ` Jeff Layton
@ 2026-10-02 15:55 ` Chuck Lever
2026-10-12 10:04 ` [PATCH v3] " Hyunsol Mun
0 siblings, 1 reply; 4+ messages in thread
From: Chuck Lever @ 2026-10-02 15:55 UTC (permalink / raw)
To: Jeff Layton, Hyunsol Mun, linux-nfs
Cc: NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey, BoB_Tobabz
On Fri, Oct 2, 2026, at 12:20 AM, Jeff Layton wrote:
> On Wed, 2026-09-30 at 14:39 +0900, Hyunsol Mun wrote:
>> A COPY_NOTIFY token can resolve to a stateid owned by the source client.
>> find_cpntf_state() drops its source-client reference before returning
>> that stateid, so concurrent source-client expiry can free sc_client
>> while stateid validation or the caller still uses the stateid.
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index bbc16dd22..43906f87e 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -2805,6 +2805,15 @@ static void __free_client(struct kref *k)
>> kmem_cache_free(client_slab, clp);
>> }
>>
>> +/**
>> + * nfsd4_get_client - acquire a reference on an nfs4_client
>> + * @clp: the client to be acquired
>> + */
>> +static void nfsd4_get_client(struct nfs4_client *clp)
>> +{
>> + kref_get(&clp->cl_nfsdfs.cl_ref);
>> +}
>> +
>
> If you're going to add this helper, then you should convert all of the
> other places that call kref_get() on this object to use it, ideally in
> a separate patch.
I applied this one to my private tree, but it conflicted with an earlier
fix, so it is modified in my tree. Perhaps I should drop it instead, and
muumthf can rebase this fix and repost, once I have refreshed the public
version of nfsd-testing in a couple of days.
Jeff, note that one of the other one-off fixes posted this week also adds
the same helper...
>> @@ -8551,6 +8564,7 @@ void nfs4_put_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
>> * @flags: flags describing type of operation to be done
>> * @nfp: optional nfsd_file return pointer (may be NULL)
>> * @cstid: optional returned nfs4_stid pointer (may be NULL)
>> + * @cstid_clp: client reference paired with @cstid (required with @cstid)
>> *
>> * Given info from the client, look up a nfs4_stid for the operation. On
>> * success, it returns a reference to the nfs4_stid and/or the nfsd_file
>> @@ -8560,10 +8574,11 @@ __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 **nfp,
>> - struct nfs4_stid **cstid)
>> + struct nfs4_stid **cstid, struct nfs4_client **cstid_clp)
>
> The bug seems possible (esp given the fault-injected reproducer), but
> this function is really devolving into a big mess. It would be very
> nice to move the COPY-specific code into a dedicated helper that is
> only called from the COPY-related codepaths. Consider reorganizing this
> code along those lines.
That's another reason I should drop this version of the fix and let
muumthf rework it.
--
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3] nfsd: pin source client while using COPY_NOTIFY stateid
2026-10-02 15:55 ` Chuck Lever
@ 2026-10-12 10:04 ` Hyunsol Mun
0 siblings, 0 replies; 4+ messages in thread
From: Hyunsol Mun @ 2026-10-12 10:04 UTC (permalink / raw)
To: linux-nfs
Cc: Chuck Lever, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
Tom Talpey, bobtobabz, Hyunsol Mun
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-12 10:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [PATCH v3] " Hyunsol Mun
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.