From: NeilBrown <neilb@ownmail.net>
To: Chuck Lever <chuck.lever@oracle.com>, Jeff Layton <jlayton@kernel.org>
Cc: Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
linux-nfs@vger.kernel.org
Subject: [PATCH v5 03/11] nfsd: simplify foreign-filehandle handling to better match RFC-7862
Date: Wed, 19 Nov 2025 14:28:49 +1100 [thread overview]
Message-ID: <20251119033204.360415-4-neilb@ownmail.net> (raw)
In-Reply-To: <20251119033204.360415-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
When the COPY v4.2 op is used in the inter-server copy mode, the file
handle of the source file (presented as the saved filehandle to COPY)
is for a different ("foreign") file server which would not be expected
to be understood by this server. fh_verify() might return nfserr_stale
or nfserr_badhandle.
In order of this filehandle to still be available to COPY, both PUTFH
and SAVEFH much not trigger an error.
RFC 7862 section 15.2.3 says:
If the request is for an inter-server copy, the source-fh is a
filehandle from the source server and the COMPOUND procedure is being
executed on the destination server. In this case, the source-fh is a
foreign filehandle on the server receiving the COPY request. If
either PUTFH or SAVEFH checked the validity of the filehandle, the
operation would likely fail and return NFS4ERR_STALE.
If a server supports the inter-server copy feature, a PUTFH followed
by a SAVEFH MUST NOT return NFS4ERR_STALE for either operation.
These restrictions do not pose substantial difficulties for servers.
CURRENT_FH and SAVED_FH may be validated in the context of the
operation referencing them and an NFS4ERR_STALE error returned for an
invalid filehandle at that point.
[The RFC neglects the possibility of NFS4ERR_BADHANDLE]
Linux nfsd currently takes a different approach. Rather than just
checking for "a PUTFH followed by a SAVEFH", it goes further to consider
only that case when this filehandle is then used for COPY, and allows
that it might have been subject of RESTOREFH and SAVEFH in between.
This is not a problem in itself except for the extra code with little
benefit. This analysis of the COMPOUND to detect PUTFH ops which need
care is performed on every COMPOUND request, which is not necessary.
It is sufficient to check if the relevant conditions are met only when a
PUTFH op actually receives an error from fh_verify().
This patch removes the checking code from common paths and places it in
nfsd4_putfh() only when fh_verify() returns a relevant error.
Rather than scanning ahead for a COPY, this patch notes (in
nfsd4_compoundargs) when an inter-server COPY is decoded, and in that
case only checks the next op to see if it is SAVEFH as this is what the
RFC requires.
A test on "inter_copy_offload_enable" is also added to be completely
consistent with the "If a server supports the inter-server copy feature"
precondition.
As we do this test in nfsd4_putfh() there is no now need to mark the
putfh op as "no_verify".
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfsd/nfs4proc.c | 64 ++++++++++++++++------------------------------
fs/nfsd/nfs4xdr.c | 2 +-
fs/nfsd/xdr4.h | 2 +-
3 files changed, 24 insertions(+), 44 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 3160e899a5da..e6f8b5b907a9 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -693,8 +693,28 @@ nfsd4_putfh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
putfh->pf_fhlen);
ret = fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_BYPASS_GSS);
#ifdef CONFIG_NFSD_V4_2_INTER_SSC
- if (ret == nfserr_stale && putfh->no_verify)
- ret = 0;
+ if ((ret == nfserr_badhandle || ret == nfserr_stale) &&
+ inter_copy_offload_enable) {
+ struct nfsd4_compoundargs *args = rqstp->rq_argp;
+ struct nfsd4_compoundres *resp = rqstp->rq_resp;
+ struct nfsd4_op *next_op = &args->ops[resp->opcnt];
+
+ if (resp->opcnt <= args->opcnt &&
+ next_op->opnum == OP_SAVEFH &&
+ args->is_inter_server_copy) {
+ /*
+ * RFC 7862 section 15.2.3 says:
+ * If a server supports the inter-server copy
+ * feature, a PUTFH followed by a SAVEFH MUST
+ * NOT return NFS4ERR_STALE for either
+ * operation.
+ * We limit this to when there is a COPY
+ * in the COMPOUND, and extend it to
+ * NFS4ERR_BADHANDLE.
+ */
+ ret = 0;
+ }
+ }
#endif
return ret;
}
@@ -2809,45 +2829,6 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
}
-#ifdef CONFIG_NFSD_V4_2_INTER_SSC
-static void
-check_if_stalefh_allowed(struct nfsd4_compoundargs *args)
-{
- struct nfsd4_op *op, *current_op = NULL, *saved_op = NULL;
- struct nfsd4_copy *copy;
- struct nfsd4_putfh *putfh;
- int i;
-
- /* traverse all operation and if it's a COPY compound, mark the
- * source filehandle to skip verification
- */
- for (i = 0; i < args->opcnt; i++) {
- op = &args->ops[i];
- if (op->opnum == OP_PUTFH)
- current_op = op;
- else if (op->opnum == OP_SAVEFH)
- saved_op = current_op;
- else if (op->opnum == OP_RESTOREFH)
- current_op = saved_op;
- else if (op->opnum == OP_COPY) {
- copy = (struct nfsd4_copy *)&op->u;
- if (!saved_op) {
- op->status = nfserr_nofilehandle;
- return;
- }
- putfh = (struct nfsd4_putfh *)&saved_op->u;
- if (nfsd4_ssc_is_inter(copy))
- putfh->no_verify = true;
- }
- }
-}
-#else
-static void
-check_if_stalefh_allowed(struct nfsd4_compoundargs *args)
-{
-}
-#endif
-
/*
* COMPOUND call.
*/
@@ -2897,7 +2878,6 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
resp->opcnt = 1;
goto encode_op;
}
- check_if_stalefh_allowed(args);
rqstp->rq_lease_breaker = (void **)&cstate->clp;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 51ef97c25456..7e44af3d10b9 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1250,7 +1250,6 @@ nfsd4_decode_putfh(struct nfsd4_compoundargs *argp, union nfsd4_op_u *u)
if (!putfh->pf_fhval)
return nfserr_jukebox;
- putfh->no_verify = false;
return nfs_ok;
}
@@ -2047,6 +2046,7 @@ nfsd4_decode_copy(struct nfsd4_compoundargs *argp, union nfsd4_op_u *u)
if (status)
return status;
+ argp->is_inter_server_copy = true;
ns_dummy = kmalloc(sizeof(struct nl4_server), GFP_KERNEL);
if (ns_dummy == NULL)
return nfserr_jukebox;
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 1f0967236cc2..3de8f4e07c49 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -335,7 +335,6 @@ struct nfsd4_lookup {
struct nfsd4_putfh {
u32 pf_fhlen; /* request */
char *pf_fhval; /* request */
- bool no_verify; /* represents foreigh fh */
};
struct nfsd4_getxattr {
@@ -907,6 +906,7 @@ struct nfsd4_compoundargs {
u32 client_opcnt;
u32 opcnt;
bool splice_ok;
+ bool is_inter_server_copy;
struct nfsd4_op *ops;
struct nfsd4_op iops[8];
};
--
2.50.0.107.gf914562f5916.dirty
next prev parent reply other threads:[~2025-11-19 3:32 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-19 3:28 [PATCH v5 00/11] nfsd: assorted cleanups involving v4 special stateids NeilBrown
2025-11-19 3:28 ` [PATCH v5 01/11] nfsd: rename ALLOWED_WITHOUT_FH to ALLOWED_WITHOUT_LOCAL_FH and revise use NeilBrown
2025-11-19 16:02 ` Chuck Lever
2025-11-19 21:13 ` NeilBrown
2025-11-19 19:12 ` Jeff Layton
2025-11-19 3:28 ` [PATCH v5 02/11] nfsd: discard NFSD4_FH_FOREIGN NeilBrown
2025-11-19 16:27 ` Chuck Lever
2025-11-19 21:25 ` NeilBrown
2025-11-19 19:13 ` Jeff Layton
2025-11-19 3:28 ` NeilBrown [this message]
2025-11-19 16:55 ` [PATCH v5 03/11] nfsd: simplify foreign-filehandle handling to better match RFC-7862 Chuck Lever
2025-11-19 21:38 ` NeilBrown
2025-11-20 21:58 ` Chuck Lever
2025-11-22 0:46 ` NeilBrown
2025-11-19 19:23 ` Jeff Layton
2025-11-19 3:28 ` [PATCH v5 04/11] nfsd: report correct error for attempt to use foreign filehandle NeilBrown
2025-11-19 19:26 ` Jeff Layton
2025-11-19 3:28 ` [PATCH v5 05/11] nfsd: drop explicit tests for special stateids which would be invalid NeilBrown
2025-11-19 19:11 ` Chuck Lever
2025-11-19 19:32 ` Jeff Layton
2025-11-19 3:28 ` [PATCH v5 06/11] nfsd: revise names of special stateid, and predicate functions NeilBrown
2025-11-19 19:27 ` Chuck Lever
2025-11-19 21:47 ` NeilBrown
2025-11-19 3:28 ` [PATCH v5 07/11] nfsd: simplify clearing of current-state-id NeilBrown
2025-11-19 20:23 ` Chuck Lever
2025-11-19 21:55 ` NeilBrown
2025-11-19 3:28 ` [PATCH v5 08/11] nfsd: simplify use of the current stateid NeilBrown
2025-11-19 3:28 ` [PATCH v5 09/11] nfsd: simplify saving " NeilBrown
2025-11-19 3:28 ` [PATCH v5 10/11] nfsd: discard current_stateid.h NeilBrown
2025-11-19 3:28 ` [PATCH v5 11/11] nfsd: conditionally clear seqid when current_stateid is used NeilBrown
2025-11-19 20:32 ` [PATCH v5 00/11] nfsd: assorted cleanups involving v4 special stateids 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=20251119033204.360415-4-neilb@ownmail.net \
--to=neilb@ownmail.net \
--cc=Dai.Ngo@oracle.com \
--cc=chuck.lever@oracle.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox