[PATCH v7 0/5] NFSD: Enforce export security after a deferred PUTFH check

[PATCH v7 0/5] NFSD: Enforce export security after a deferred PUTFH check

[Chuck Lever]

[ I was hoping v6 would be final, but v7 is needed to address
  some reasonable agentic review findings. ]

VeroFess reported that an AUTH_SYS NFSv4.0 client can read or write
a file on a sec=krb5p export with another client's OPEN stateid by
placing SAVEFH between PUTFH and the I/O operation. The v4 posting
carried that fix together with changes to COPY, LOOKUP, READDIR, and
SECINFO in one patch, which would not apply to the stable kernels
that need the fix. This series splits that patch. The
need_wrongsec_check() change stands alone (patch 1) and applies to
every LTS kernel, and the remaining patches carry only the Fixes:
tags that match their code. VeroFess developed the original fix with
the assistance of an LLM, and an LLM assisted with the split and the
commit messages in this series.

VeroFess tested v5 incrementally against the reproducer in a KASAN
guest, with an RPC-over-TLS run of the full series, and the first
three patches carry a Tested-by. The SECINFO and PUTFH patches
changed in v7 and have not been run against a krb5 or TLS client.

After this series, PUTFH checks the export's transport policy but not
its security flavor, for any RPC flavor. need_wrongsec_check() reports
NFS4ERR_WRONGSEC from PUTFH when the next operation uses the current
filehandle, and LOOKUP, LOOKUPP, OPEN, and SECINFO verify the
filehandle themselves. A sec=sys client reading a pseudo-root that
has a sec=krb5p child now receives rdattr_error for that entry
instead of the child's attributes.

---
Changes in v7:
- Skip the PUTFH flavor check for every flavor (sashiko).
- Split the mount-crossing fix out of the SECINFO patch.
- Carry only Fixes: f0d6ad5a2f67 on the mount-crossing patch.
- Cite RFC 8881 Section 18.29.4 for READDIR's rdattr_error.
- Rename NFSD_MAY_SECINFO to NFSD_MAY_BYPASS_FLAVOR.
- Drop the PUTFH limitation paragraph from the SECINFO description.
- Link to v6: https://patch.msgid.link/20261101-verofess-v6-0-4d70097db7d3@kernel.org

Changes in v6:
- Verify the current filehandle in SECINFO_NO_NAME(CURRENT_FH) (sashiko).
- Note that PUTFH still returns WRONGSEC for a GSS or xprtsec= mismatch.
- Link to v5: https://patch.msgid.link/20261030-verofess-v5-0-242eea2991f3@kernel.org

Changes in v5:
- Split the single patch into three, each with its own Fixes: tags.
- Drop the nfs4_check_file() and COPY_NOTIFY export checks.
- Check the entered export in nfsd_cross_mnt() and nfsd_lookup_parent().
- Replace the for_secinfo flag with NFSD_MAY_SECINFO.
- Report a SECINFO transport policy mismatch as NFS4ERR_ACCESS.
- Keep the PUTFH export check for NFSv4.0 PUTFH; SECINFO.
- Link to v4: https://lore.kernel.org/r/20261029063917.138184-1-coderzeng@gmail.com

Changes in v4:
- Skip only the flavor check for SECINFO; xprtsec= stays enforced.
- Re-check xprtsec= when a SECINFO lookup selects another export.
- Link to v3: https://lore.kernel.org/r/20261029051430.96326-1-coderzeng@gmail.com

Changes in v3:
- Use nfsd_lookup_dentry() for SECINFO_NO_NAME(PARENT).
- Let SECINFO and SECINFO_NO_NAME(PARENT) query a strict starting export.
- Skip the source export's policy check on a SECINFO mount crossing.
- Link to v2: https://lore.kernel.org/r/20261029013648.12015-1-coderzeng@gmail.com

Changes in v2:
- Check cached stateid I/O and COPY_NOTIFY against the current export.
- Check the selected export after LOOKUP and READDIR mount crossing.
- Map READDIR's NFS4ERR_WRONGSEC to NFS4ERR_ACCESS without rdattr_error.
- Add the missing Fixes: tags and regenerate the whitespace-damaged diff.
- v1 was sent off-list.

---
Chuck Lever (1):
      NFSD: Defer the PUTFH export security flavor check

VeroFess (4):
      NFSD: Fix PUTFH wrongsec check when SAVEFH follows
      NFSD: Check export security on the COPY and CLONE source
      NFSD: Check the entered export on a mount crossing
      NFSD: Let SECINFO query an export that excludes the request's flavor

 fs/nfsd/export.c    |  6 ++--
 fs/nfsd/nfs3proc.c  |  2 +-
 fs/nfsd/nfs4proc.c  | 87 ++++++++++++++++++++++++++++++++++++++---------------
 fs/nfsd/nfs4state.c |  2 +-
 fs/nfsd/nfs4xdr.c   |  8 +++--
 fs/nfsd/nfsfh.c     |  9 ++++--
 fs/nfsd/nfsproc.c   |  2 +-
 fs/nfsd/trace.h     |  3 +-
 fs/nfsd/vfs.c       | 72 +++++++++++++++++++++++++-------------------
 fs/nfsd/vfs.h       | 43 ++++++++++++++++++++++----
 10 files changed, 161 insertions(+), 73 deletions(-)
---
base-commit: d9130c9f6535392d423351ffb5521e7689cae6b6
change-id: 20261030-verofess-f0281be339bc

Best regards,
--  
Chuck Lever <cel@kernel.org>

[PATCH v7 1/5] NFSD: Fix PUTFH wrongsec check when SAVEFH follows

[Chuck Lever]

From: VeroFess <coderzeng@gmail.com>

An AUTH_SYS NFSv4.0 client can read or write a file on a sec=krb5p
export using another client's valid OPEN stateid by inserting
SAVEFH between PUTFH and READ or WRITE.

PUTFH resolves the handle with NFSD_MAY_BYPASS_GSS and defers the
security flavor check to need_wrongsec_check(), which looks only at
the next operation. SAVEFH carries OP_HANDLES_WRONGSEC even though
it only saves the handle, so PUTFH skips NFS4ERR_WRONGSEC. The READ
or WRITE that follows then reuses a cached nfsd_file.

With these export options:

  rw,sync,no_subtree_check,fsid=0,sec=krb5p,secure,root_squash

a Kerberos privacy NFSv4.0 client held an OPEN stateid. With the
same stateid, PUTFH; READ over AUTH_SYS was rejected, but PUTFH;
SAVEFH; READ and PUTFH; SAVEFH; WRITE succeeded, and the Kerberos
client subsequently read the changed contents.

SAVEFH leaves CURRENT_FH in place. When deciding whether PUTFH must
report NFS4ERR_WRONGSEC, look past SAVEFH to the first operation
that uses or replaces CURRENT_FH. If another PUTFH replaces it,
leave the earlier PUTFH unchecked, as RFC 8881 Section 2.6.3.1.1
requires. Drop OP_HANDLES_WRONGSEC from SAVEFH, which performs no
check of its own and is never consulted once it is skipped.

Fixes: 68d93184352f ("nfsd4: fix wrongsec handling for PUTFH + op cases")
Signed-off-by: VeroFess <coderzeng@gmail.com>
Tested-by: VeroFess <coderzeng@gmail.com>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4proc.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index d0290fce9187..50f38d6e147b 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -3237,32 +3237,39 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
 	struct nfsd4_compoundres *resp = rqstp->rq_resp;
 	struct nfsd4_compoundargs *argp = rqstp->rq_argp;
 	struct nfsd4_op *this = &argp->ops[resp->opcnt - 1];
-	struct nfsd4_op *next = &argp->ops[resp->opcnt];
+	struct nfsd4_op *next = NULL;
 	const struct nfsd4_operation *thisd = OPDESC(this);
 	const struct nfsd4_operation *nextd;
+	unsigned int i;
 
-	/*
-	 * Most ops check wronsec on our own; only the putfh-like ops
-	 * have special rules.
-	 */
+	/* Only put-filehandle operations need a deferred WRONGSEC check. */
 	if (!(thisd->op_flags & OP_IS_PUTFH_LIKE))
 		return false;
+
 	/*
-	 * rfc 5661 2.6.3.1.1.6: don't bother erroring out a
-	 * put-filehandle operation if we're not going to use the
-	 * result:
+	 * RFC 8881 Section 2.6.3.1.1.1: SAVEFH does not count as the
+	 * next operation.
 	 */
-	if (argp->opcnt == resp->opcnt)
+	for (i = resp->opcnt; i < argp->opcnt; i++) {
+		next = &argp->ops[i];
+		if (next->opnum != OP_SAVEFH)
+			break;
+	}
+
+	/* RFC 8881 Section 2.6.3.1.1.6: nothing uses the filehandle. */
+	if (i == argp->opcnt)
 		return false;
+
 	if (next->opnum == OP_ILLEGAL)
 		return false;
 	nextd = OPDESC(next);
 	/*
-	 * Rest of 2.6.3.1.1: certain operations will return WRONGSEC
-	 * errors themselves as necessary; others should check for them
-	 * now:
+	 * RFC 8881 Section 2.6.3.1.1.2: a later put-filehandle operation
+	 * replaces this one. Other marked operations check WRONGSEC
+	 * themselves.
 	 */
-	return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
+	return !(nextd->op_flags &
+		  (OP_HANDLES_WRONGSEC | OP_IS_PUTFH_LIKE));
 }
 
 #ifdef CONFIG_NFSD_V4_2_INTER_SSC
@@ -4047,7 +4054,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_SAVEFH] = {
 		.op_func = nfsd4_savefh,
-		.op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
+		.op_flags = OP_MODIFIES_SOMETHING,
 		.op_name = "OP_SAVEFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},

-- 
2.56.0

[PATCH v7 2/5] NFSD: Check export security on the COPY and CLONE source

[Chuck Lever]

From: VeroFess <coderzeng@gmail.com>

RFC 8881 Section 2.6.3.1.1 forbids PUTFH from returning
NFS4ERR_WRONGSEC when another PUTFH follows, so in PUTFH; SAVEFH;
PUTFH; COPY the source handle reaches nfsd4_verify_copy() with its
export's security flavor unchecked. The source stateid then finds a
cached nfsd_file and the copy proceeds.

On a kernel with only the need_wrongsec_check() fix, an AUTH_SYS
READ of a sec=krb5p source returned NFS4ERR_WRONGSEC, but PUTFH;
SAVEFH; PUTFH; COPY of the same file completed and wrote the source
bytes to the destination.

Check the saved export with check_nfsd_access() before using the
source stateid. Map NFS4ERR_WRONGSEC to NFS4ERR_ACCESS, since
neither COPY nor CLONE permits NFS4ERR_WRONGSEC.

Fixes: 29ae7f9dc21a ("NFSD: Implement the COPY call")
Signed-off-by: VeroFess <coderzeng@gmail.com>
Tested-by: VeroFess <coderzeng@gmail.com>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4proc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 50f38d6e147b..b6dcb889b5eb 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1491,6 +1491,11 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (!cstate->save_fh.fh_dentry)
 		return nfserr_nofilehandle;
 
+	/* A cached open stateid does not authorize the saved source export. */
+	status = check_nfsd_access(cstate->save_fh.fh_export, rqstp);
+	if (status)
+		return status == nfserr_wrongsec ? nfserr_acces : status;
+
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
 					    src_stateid, RD_STATE, src, NULL);
 	if (status)

-- 
2.56.0

[PATCH v7 3/5] NFSD: Check the entered export on a mount crossing

[Chuck Lever]

From: VeroFess <coderzeng@gmail.com>

A bind-mounted regular file can have a stricter export than its
parent. nfsd_cross_mnt() checks the request's flavor against the
export being left, not the one being entered, so LOOKUP, READDIR, and
the NFSv2 and NFSv3 CREATE paths all hand out filehandles on the child
export without consulting its sec= list. Stepping up with ".." has the
same gap: nfsd_lookup_parent() switches to the parent export without
any check, so LOOKUPP from the root of a sec=sys export returns a
filehandle on a sec=krb5p parent. Every caller has already passed
fh_verify() on the export being left, so check only the export being
entered, in nfsd_cross_mnt() and in nfsd_lookup_parent().

READDIR crosses into child exports to fetch entry attributes, so the
new check also fires there. When the client requests rdattr_error,
report NFS4ERR_WRONGSEC in that attribute, as RFC 8881 Section 18.29.4
describes. Otherwise the whole READDIR fails, as Section 18.23.3
requires. NFS4ERR_WRONGSEC is not a READDIR status, so report
NFS4ERR_ACCESS. The Linux, FreeBSD, illumos, and Windows clients all
request rdattr_error on every READDIR. A client reading the pseudo-root
with a flavor that a child export excludes now receives rdattr_error
for that entry instead of the child's attributes.

Fixes: f0d6ad5a2f67 ("nfsd: move check_nfsd_access() call into nfsd_cross_mnt()")
Signed-off-by: VeroFess <coderzeng@gmail.com>
Tested-by: VeroFess <coderzeng@gmail.com>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4xdr.c |  6 +++++-
 fs/nfsd/vfs.c     | 32 +++++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index d4a88bef8444..a6651349bcc8 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4775,8 +4775,12 @@ nfsd4_encode_entry4(void *ccdv, const char *name, int namlen,
 		 * then in accordance with the spec, we fail the
 		 * entire READDIR operation(!)
 		 */
-		if (!(cd->rd_bmval[0] & FATTR4_WORD0_RDATTR_ERROR))
+		if (!(cd->rd_bmval[0] & FATTR4_WORD0_RDATTR_ERROR)) {
+			/* READDIR cannot return NFS4ERR_WRONGSEC itself. */
+			if (nfserr == nfserr_wrongsec)
+				nfserr = nfserr_acces;
 			goto fail;
+		}
 		if (nfsd4_encode_entry4_rdattr_error(xdr, nfserr)) {
 			nfserr = nfserr_toosmall;
 			goto fail;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index a8cf41e4dfea..3486a2ee26ba 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -169,7 +169,7 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
 			err = 0;
 	} else if (nfsd_v4client(rqstp) ||
 		(exp->ex_flags & NFSEXP_CROSSMOUNT) || EX_NOHIDE(exp2)) {
-		nfserr = check_nfsd_access(exp, rqstp);
+		nfserr = check_nfsd_access(exp2, rqstp);
 		if (nfserr == nfs_ok) {
 			/* successfully crossed mount point */
 			/*
@@ -204,11 +204,13 @@ static void follow_to_parent(struct path *path)
 	path->dentry = dp;
 }
 
-static int nfsd_lookup_parent(struct svc_rqst *rqstp, struct dentry *dparent, struct svc_export **exp, struct dentry **dentryp)
+static __be32 nfsd_lookup_parent(struct svc_rqst *rqstp, struct dentry *dparent,
+				 struct svc_export **exp, struct dentry **dentryp)
 {
 	struct svc_export *exp2;
 	struct path path = {.mnt = mntget((*exp)->ex_path.mnt),
 			    .dentry = dget(dparent)};
+	__be32 nfserr = nfs_ok;
 
 	follow_to_parent(&path);
 
@@ -216,15 +218,19 @@ static int nfsd_lookup_parent(struct svc_rqst *rqstp, struct dentry *dparent, st
 	if (PTR_ERR(exp2) == -ENOENT) {
 		*dentryp = dget(dparent);
 	} else if (IS_ERR(exp2)) {
-		path_put(&path);
-		return PTR_ERR(exp2);
+		nfserr = nfserrno(PTR_ERR(exp2));
 	} else {
-		*dentryp = dget(path.dentry);
-		exp_put(*exp);
-		*exp = exp2;
+		nfserr = check_nfsd_access(exp2, rqstp);
+		if (nfserr != nfs_ok) {
+			exp_put(exp2);
+		} else {
+			*dentryp = dget(path.dentry);
+			exp_put(*exp);
+			*exp = exp2;
+		}
 	}
 	path_put(&path);
-	return 0;
+	return nfserr;
 }
 
 /*
@@ -277,9 +283,13 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 			dentry = dget(dparent); /* .. == . just like at / */
 		else {
 			/* checking mountpoint crossing is very different when stepping up */
-			host_err = nfsd_lookup_parent(rqstp, dparent, &exp, &dentry);
-			if (host_err)
-				goto out_nfserr;
+			__be32 nfserr = nfsd_lookup_parent(rqstp, dparent,
+							   &exp, &dentry);
+
+			if (nfserr) {
+				exp_put(exp);
+				return nfserr;
+			}
 		}
 	} else {
 		dentry = lookup_one_unlocked(&nop_mnt_idmap,

-- 
2.56.0

[PATCH v7 4/5] NFSD: Let SECINFO query an export that excludes the request's flavor

[Chuck Lever]

From: VeroFess <coderzeng@gmail.com>

SECINFO and SECINFO_NO_NAME report export security policies. RFC
8881 Section 2.6.3.1.1.5 forbids NFS4ERR_WRONGSEC from either
operation, which allows a client using any flavor to learn whether
a name exists in a directory on a sec=krb5p export. The starting
directory's fh_verify(..., NFSD_MAY_EXEC) call rejects SECINFO and
SECINFO_NO_NAME(PARENT) when that export does not allow the request's
flavor, and a mount crossing rejects it again on the crossed-into
export. SECINFO_NO_NAME(CURRENT_FH) never calls fh_verify() at all, so
after PUTROOTFH it reports the pseudo-root's flavors without checking
that export's transport policy or secure port requirement.

Add NFSD_MAY_BYPASS_FLAVOR, which fh_verify() and the lookup path
both read: it skips the starting export's security flavor check and
the flavor check on a mount crossing or parent step-up. Verify the
current filehandle with it in SECINFO_NO_NAME(CURRENT_FH) as well.
NFSv4.0 is different: RFC 7530 permits NFS4ERR_WRONGSEC from PUTFH and
from neither SECINFO nor the operation after it, and a v4.0 SECINFO
leaves the current filehandle in place. A v4.0 put-filehandle operation
followed by SECINFO therefore keeps its own export check, so the error
is reported where RFC 7530 allows it. The Linux and illumos clients
send SECINFO only with a parent directory they already hold under the
same flavor, and FreeBSD does not send SECINFO at all, so the check does
not fire for any of them. The export's transport policy (xprtsec=) is a
transport-layer requirement, not part of the flavor negotiation SECINFO
performs, so SECINFO still enforces it on the starting export and on
any export a crossing enters. When SECINFO or SECINFO_NO_NAME itself
detects the transport policy mismatch, report NFS4ERR_ACCESS, the error
RFC 8881 Section 18.29.3 specifies for a SECINFO access failure. The
directory type, secure-port, and execute-permission checks on the
starting directory still apply.

Fixes: 29a78a3ed7fc ("nfsd4: make fh_verify responsibility of nfsd_lookup_dentry caller")
Fixes: 04f4ad16b231 ("nfsd4: implement secinfo_no_name")
Signed-off-by: VeroFess <coderzeng@gmail.com>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/export.c    |  6 ++++--
 fs/nfsd/nfs3proc.c  |  2 +-
 fs/nfsd/nfs4proc.c  | 44 ++++++++++++++++++++++++++++++++++----------
 fs/nfsd/nfs4state.c |  2 +-
 fs/nfsd/nfs4xdr.c   |  2 +-
 fs/nfsd/nfsfh.c     |  9 ++++++---
 fs/nfsd/nfsproc.c   |  2 +-
 fs/nfsd/trace.h     |  3 ++-
 fs/nfsd/vfs.c       | 48 ++++++++++++++++++++++++------------------------
 fs/nfsd/vfs.h       | 43 +++++++++++++++++++++++++++++++++++++------
 10 files changed, 111 insertions(+), 50 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 397767d756c6..f0dd9fb22ec2 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1976,8 +1976,10 @@ static struct svc_export *exp_find(struct cache_detail *cd,
  *
  * Helper function for check_nfsd_access().  Note that callers should be
  * using check_nfsd_access() instead of calling this function directly.  The
- * one exception is __fh_verify() since it has logic that may result in one
- * or both of the helpers being skipped.
+ * exceptions are __fh_verify(), which has logic that may result in one
+ * or both of the helpers being skipped, and the mount-crossing paths in
+ * nfsd_cross_mnt() and nfsd_lookup_parent(), which enforce only the
+ * transport policy when NFSD_MAY_BYPASS_FLAVOR is set.
  *
  * Return values:
  *   %nfs_ok if access is granted, or
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 11e05b2e5e84..1025cec447a9 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -961,7 +961,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *dirfhp,
 		 */
 		if (d_is_reg(child) &&
 		    unlikely(nfsd_mountpoint(child, exp))) {
-			status = nfsd_cross_mnt(rqstp, &child, &exp);
+			status = nfsd_cross_mnt(rqstp, &child, &exp, 0);
 			if (status != nfs_ok) {
 				exp_put(exp);
 				goto out;
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b6dcb889b5eb..6ff59fac9094 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -342,7 +342,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		    unlikely(nfsd_mountpoint(child, fhp->fh_export))) {
 			struct svc_export *exp = exp_get(fhp->fh_export);
 
-			status = nfsd_cross_mnt(rqstp, &child, &exp);
+			status = nfsd_cross_mnt(rqstp, &child, &exp, 0);
 			if (status == nfs_ok)
 				status = fh_compose(resfhp, exp,
 						    child, fhp);
@@ -1079,7 +1079,17 @@ nfsd4_link(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	return status;
 }
 
-static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
+/*
+ * RFC 8881 Section 2.6.3.1.1.5 forbids NFS4ERR_WRONGSEC from SECINFO.
+ * Section 18.29.3 has SECINFO report a denied lookup as NFS4ERR_ACCESS.
+ */
+static __be32 nfsd4_secinfo_status(__be32 status)
+{
+	return status == nfserr_wrongsec ? nfserr_acces : status;
+}
+
+static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh,
+			       int access)
 {
 	struct svc_fh tmp_fh;
 	__be32 ret;
@@ -1093,14 +1103,14 @@ static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
 		return nfserr_noent;
 	}
 	fh_put(&tmp_fh);
-	return nfsd_lookup(rqstp, fh, "..", 2, fh);
+	return __nfsd_lookup(rqstp, fh, "..", 2, fh, access);
 }
 
 static __be32
 nfsd4_lookupp(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	      union nfsd4_op_u *u)
 {
-	return nfsd4_do_lookupp(rqstp, &cstate->current_fh);
+	return nfsd4_do_lookupp(rqstp, &cstate->current_fh, NFSD_MAY_EXEC);
 }
 
 static __be32
@@ -1252,14 +1262,16 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct dentry *dentry;
 	__be32 err;
 
-	err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, NFSD_MAY_EXEC);
+	err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR,
+			NFSD_MAY_EXEC | NFSD_MAY_BYPASS_FLAVOR);
 	if (err)
-		return err;
+		return nfsd4_secinfo_status(err);
 	err = nfsd_lookup_dentry(rqstp, &cstate->current_fh,
 				    secinfo->si_name, secinfo->si_namelen,
-				    &exp, &dentry);
+				    &exp, &dentry,
+				    NFSD_MAY_EXEC | NFSD_MAY_BYPASS_FLAVOR);
 	if (err)
-		return err;
+		return nfsd4_secinfo_status(err);
 	if (d_really_is_negative(dentry)) {
 		exp_put(exp);
 		err = nfserr_noent;
@@ -1280,11 +1292,16 @@ nfsd4_secinfo_no_name(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstat
 
 	switch (u->secinfo_no_name.sin_style) {
 	case NFS4_SECINFO_STYLE4_CURRENT_FH:
+		err = fh_verify(rqstp, &cstate->current_fh, 0,
+				NFSD_MAY_BYPASS_FLAVOR);
+		if (err)
+			return nfsd4_secinfo_status(err);
 		break;
 	case NFS4_SECINFO_STYLE4_PARENT:
-		err = nfsd4_do_lookupp(rqstp, &cstate->current_fh);
+		err = nfsd4_do_lookupp(rqstp, &cstate->current_fh,
+				       NFSD_MAY_EXEC | NFSD_MAY_BYPASS_FLAVOR);
 		if (err)
-			return err;
+			return nfsd4_secinfo_status(err);
 		break;
 	default:
 		return nfserr_inval;
@@ -3267,6 +3284,13 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
 
 	if (next->opnum == OP_ILLEGAL)
 		return false;
+	/*
+	 * RFC 7530 permits NFS4ERR_WRONGSEC from PUTFH but not from
+	 * SECINFO, and NFSv4.0 SECINFO leaves the current filehandle in
+	 * place for the next operation.
+	 */
+	if (resp->cstate.minorversion == 0 && next->opnum == OP_SECINFO)
+		return true;
 	nextd = OPDESC(next);
 	/*
 	 * RFC 8881 Section 2.6.3.1.1.2: a later put-filehandle operation
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cafdf80f32de..97b4fa30362a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7126,7 +7126,7 @@ nfsd4_verify_deleg_dentry(struct nfsd4_open *open, struct nfs4_file *fp,
 
 	err = nfsd_lookup_dentry(open->op_rqstp, parent,
 				 open->op_fname, open->op_fnamelen,
-				 &exp, &child);
+				 &exp, &child, 0);
 
 	if (err)
 		return -ESTALE;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index a6651349bcc8..111bf2b8ebd1 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4676,7 +4676,7 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
 		 * Different "."/".." handling?  Something else?
 		 * At least, add a comment here to explain....
 		 */
-		nfserr = nfsd_cross_mnt(cd->rd_rqstp, &dentry, &exp);
+		nfserr = nfsd_cross_mnt(cd->rd_rqstp, &dentry, &exp, 0);
 		if (nfserr)
 			goto out_put;
 		crossed = true;
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index f1ba5a14bb85..09391464803f 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -471,9 +471,12 @@ __fh_verify(struct svc_rqst *rqstp,
 			&& exp->ex_path.dentry == dentry)
 		may_bypass_gss = true;
 
-	error = check_security_flavor(exp, rqstp, may_bypass_gss);
-	if (error)
-		goto out;
+	/* The xprtsec= policy applies even when the flavor check is bypassed. */
+	if (!(access & NFSD_MAY_BYPASS_FLAVOR)) {
+		error = check_security_flavor(exp, rqstp, may_bypass_gss);
+		if (error)
+			goto out;
+	}
 
 	svc_xprt_set_valid(rqstp->rq_xprt);
 
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 2f8da62ad6b0..43e6a4b6a259 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -712,7 +712,7 @@ nfsd_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	exp = exp_get(fhp->fh_export);
 	if (d_is_reg(dchild) &&
 	    unlikely(nfsd_mountpoint(dchild, exp))) {
-		status = nfsd_cross_mnt(rqstp, &dchild, &exp);
+		status = nfsd_cross_mnt(rqstp, &dchild, &exp, 0);
 		if (status != nfs_ok) {
 			exp_put(exp);
 			goto out;
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index e7b186307c21..e58b753c1ebc 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -142,7 +142,8 @@ DEFINE_NFSD_DYNTHREAD_EVENT(trylock_fail);
 		{ NFSD_MAY_BYPASS_GSS,		"BYPASS_GSS" },		\
 		{ NFSD_MAY_READ_IF_EXEC,	"READ_IF_EXEC" },	\
 		{ NFSD_MAY_64BIT_COOKIE,	"64BIT_COOKIE" },	\
-		{ NFSD_MAY_LOCALIO,		"LOCALIO" })
+		{ NFSD_MAY_LOCALIO,		"LOCALIO" },	\
+		{ NFSD_MAY_BYPASS_FLAVOR,	"BYPASS_FLAVOR" })
 
 TRACE_EVENT(nfsd_compound,
 	TP_PROTO(
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 3486a2ee26ba..269dc4968f19 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -124,14 +124,13 @@ nfserrno(int errno)
 }
 
 /*
- * Called from nfsd_lookup and encode_dirent. Check if we have crossed
- * a mount point.
+ * Check if we have crossed a mount point.
  * Returns an nfs error leaving *dpp and *expp unchanged,
  *  or nfs_ok having possibly changed *dpp and *expp
  */
 __be32
 nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
-	       struct svc_export **expp)
+	       struct svc_export **expp, int access)
 {
 	struct svc_export *exp = *expp, *exp2 = NULL;
 	struct dentry *dentry = *dpp;
@@ -169,7 +168,9 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
 			err = 0;
 	} else if (nfsd_v4client(rqstp) ||
 		(exp->ex_flags & NFSEXP_CROSSMOUNT) || EX_NOHIDE(exp2)) {
-		nfserr = check_nfsd_access(exp2, rqstp);
+		nfserr = (access & NFSD_MAY_BYPASS_FLAVOR) ?
+			 check_xprtsec_policy(exp2, rqstp) :
+			 check_nfsd_access(exp2, rqstp);
 		if (nfserr == nfs_ok) {
 			/* successfully crossed mount point */
 			/*
@@ -205,7 +206,8 @@ static void follow_to_parent(struct path *path)
 }
 
 static __be32 nfsd_lookup_parent(struct svc_rqst *rqstp, struct dentry *dparent,
-				 struct svc_export **exp, struct dentry **dentryp)
+				 struct svc_export **exp, struct dentry **dentryp,
+				 int access)
 {
 	struct svc_export *exp2;
 	struct path path = {.mnt = mntget((*exp)->ex_path.mnt),
@@ -220,7 +222,9 @@ static __be32 nfsd_lookup_parent(struct svc_rqst *rqstp, struct dentry *dparent,
 	} else if (IS_ERR(exp2)) {
 		nfserr = nfserrno(PTR_ERR(exp2));
 	} else {
-		nfserr = check_nfsd_access(exp2, rqstp);
+		nfserr = (access & NFSD_MAY_BYPASS_FLAVOR) ?
+			 check_xprtsec_policy(exp2, rqstp) :
+			 check_nfsd_access(exp2, rqstp);
 		if (nfserr != nfs_ok) {
 			exp_put(exp2);
 		} else {
@@ -261,7 +265,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
 __be32
 nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		   const char *name, unsigned int len,
-		   struct svc_export **exp_ret, struct dentry **dentry_ret)
+		   struct svc_export **exp_ret, struct dentry **dentry_ret,
+		   int access)
 {
 	struct svc_export	*exp;
 	struct dentry		*dparent;
@@ -284,7 +289,7 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		else {
 			/* checking mountpoint crossing is very different when stepping up */
 			__be32 nfserr = nfsd_lookup_parent(rqstp, dparent,
-							   &exp, &dentry);
+							   &exp, &dentry, access);
 
 			if (nfserr) {
 				exp_put(exp);
@@ -298,7 +303,8 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		if (IS_ERR(dentry))
 			goto out_nfserr;
 		if (nfsd_mountpoint(dentry, exp)) {
-			__be32 nfserr = nfsd_cross_mnt(rqstp, &dentry, &exp);
+			__be32 nfserr = nfsd_cross_mnt(rqstp, &dentry, &exp,
+						       access);
 
 			if (nfserr) {
 				dput(dentry);
@@ -317,37 +323,31 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 }
 
 /**
- * nfsd_lookup - look up a single path component for nfsd
+ * __nfsd_lookup - look up a single path component for nfsd
  *
  * @rqstp:   the request context
  * @fhp:     the file handle of the directory
  * @name:    the component name, or %NULL to look up parent
  * @len:     length of name to examine
  * @resfh:   pointer to pre-initialised filehandle to hold result.
+ * @access:  NFSD_MAY_* flags for the fh_verify() of @fhp; adding
+ *	     NFSD_MAY_BYPASS_FLAVOR skips the export security flavor checks
  *
- * Look up one component of a pathname.
- * N.B. After this call _both_ fhp and resfh need an fh_put
- *
- * If the lookup would cross a mountpoint, and the mounted filesystem
- * is exported to the client with NFSEXP_NOHIDE, then the lookup is
- * accepted as it stands and the mounted directory is
- * returned. Otherwise the covered directory is returned.
- * NOTE: this mountpoint crossing is not supported properly by all
- *   clients and is explicitly disallowed for NFSv3
- *
+ * Variant of nfsd_lookup() that takes the fh_verify() access flags.
+ * The filehandle and mountpoint crossing rules are documented there.
  */
 __be32
-nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
-	    unsigned int len, struct svc_fh *resfh)
+__nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
+	      unsigned int len, struct svc_fh *resfh, int access)
 {
 	struct svc_export	*exp;
 	struct dentry		*dentry;
 	__be32 err;
 
-	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
+	err = fh_verify(rqstp, fhp, S_IFDIR, access);
 	if (err)
 		return err;
-	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
+	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry, access);
 	if (err)
 		return err;
 	/*
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 3cac25adf16b..939124de6bd7 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -33,6 +33,7 @@
 #define NFSD_MAY_64BIT_COOKIE		0x1000 /* 64 bit readdir cookies for >= NFSv3 */
 
 #define NFSD_MAY_LOCALIO		0x2000 /* for tracing, reflects when localio used */
+#define NFSD_MAY_BYPASS_FLAVOR		0x4000 /* skip the export's sec= check */
 
 #define NFSD_MAY_CREATE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE)
 #define NFSD_MAY_REMOVE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
@@ -128,12 +129,42 @@ void		nfsd_maybe_reset_write_verifier(struct nfsd_net *nn,
 						struct svc_rqst *rqstp,
 						int err);
 __be32		nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
-			       struct svc_export **expp);
-__be32		nfsd_lookup(struct svc_rqst *, struct svc_fh *,
-				const char *, unsigned int, struct svc_fh *);
-__be32		 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *,
-				const char *, unsigned int,
-				struct svc_export **, struct dentry **);
+			       struct svc_export **expp, int access);
+__be32		__nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp,
+			      const char *name, unsigned int len,
+			      struct svc_fh *resfh, int access);
+
+/**
+ * nfsd_lookup - look up a single path component for nfsd
+ *
+ * @rqstp:   the request context
+ * @fhp:     the file handle of the directory
+ * @name:    the component name, or %NULL to look up parent
+ * @len:     length of name to examine
+ * @resfh:   pointer to pre-initialised filehandle to hold result.
+ *
+ * Look up one component of a pathname.
+ * N.B. After this call _both_ fhp and resfh need an fh_put
+ *
+ * If the lookup would cross a mountpoint, and the mounted filesystem
+ * is exported to the client with NFSEXP_NOHIDE, then the lookup is
+ * accepted as it stands and the mounted directory is
+ * returned. Otherwise the covered directory is returned.
+ * NOTE: this mountpoint crossing is not supported properly by all
+ *   clients and is explicitly disallowed for NFSv3
+ *
+ */
+static inline __be32 nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp,
+				 const char *name, unsigned int len,
+				 struct svc_fh *resfh)
+{
+	return __nfsd_lookup(rqstp, fhp, name, len, resfh, NFSD_MAY_EXEC);
+}
+
+__be32		nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
+				   const char *name, unsigned int len,
+				   struct svc_export **exp_ret,
+				   struct dentry **dentry_ret, int access);
 __be32		nfsd_setattr(struct svc_rqst *, struct svc_fh *,
 			     struct nfsd_attrs *, const struct timespec64 *);
 int nfsd_mountpoint(struct dentry *, struct svc_export *);

-- 
2.56.0

[PATCH v7 5/5] NFSD: Defer the PUTFH export security flavor check

[Chuck Lever]

An NFSv4.1 client learns an export's security policy by sending
PUTFH; SECINFO under whatever flavor it already holds. nfsd4_putfh()
verifies the filehandle with NFSD_MAY_BYPASS_GSS, which waives the
export's flavor check only for AUTH_NULL and AUTH_SYS, so a krb5 or
krb5i request against a sec=krb5p export gets NFS4ERR_WRONGSEC from
PUTFH and SECINFO never runs. RFC 8881 Section 2.6.3.1.1.5 forbids
NFS4ERR_WRONGSEC from a put-filehandle operation that SECINFO or
SECINFO_NO_NAME follows, and Sections 2.6.3.1.1.2 through 2.6.3.1.1.4
forbid it when another put-filehandle operation, LOOKUP, LOOKUPP, or
OPEN by component name follows. Commit bb4f07f2409c ("nfsd: Fix
NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT") narrowed the
flag from skipping the check to waiving it for the two unauthenticated
flavors. That is right for the NFSv2 and NFSv3 mount-time callers, but
it also closed the path that let a GSS-authenticated PUTFH precede
those operations.

Skip the flavor check in PUTFH for every flavor. fh_verify() has
resolved the filehandle before the flavor check runs, and
need_wrongsec_check() already decides, from the operation that
follows, whether PUTFH reports NFS4ERR_WRONGSEC itself. The operations
that may follow without that check verify the current filehandle on
their own: LOOKUP, LOOKUPP, and OPEN by name through fh_verify() on
the directory, OPEN by filehandle through do_open_permission(), and
SECINFO and SECINFO_NO_NAME with NFSD_MAY_BYPASS_FLAVOR. PUTROOTFH and
RESTOREFH already rely on the deferred check alone. A v4.0 PUTFH that
SECINFO follows keeps its check, where RFC 7530 permits the error. The
transport policy check stays in PUTFH.

Fixes: bb4f07f2409c ("nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20261101-verofess-v6-0-4d70097db7d3@kernel.org?part=3
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4proc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 6ff59fac9094..2c653027d578 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -827,7 +827,8 @@ nfsd4_putfh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	cstate->current_fh.fh_handle.fh_size = putfh->pf_fhlen;
 	memcpy(&cstate->current_fh.fh_handle.fh_raw, putfh->pf_fhval,
 	       putfh->pf_fhlen);
-	ret = fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_BYPASS_GSS);
+	/* need_wrongsec_check() decides whether the flavor check applies. */
+	ret = fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_BYPASS_FLAVOR);
 #ifdef CONFIG_NFSD_V4_2_INTER_SSC
 	if (ret == nfserr_stale && putfh->no_verify) {
 		SET_FH_FLAG(&cstate->current_fh, NFSD4_FH_FOREIGN);

-- 
2.56.0

Re: [PATCH v7 0/5] NFSD: Enforce export security after a deferred PUTFH check

[VeroFess]

Chuck,

I finished testing the posted v7 series on an x86-64 KASAN guest with
NFSv2/v3/v4, Btrfs, inter-server COPY, and kernel TLS enabled. The
five patches applied in order to d9130c9f6535 and the bzImage built.
The tested image has SHA-256
99c8dad605aba384985097b6fad00f26db19ea8074b5b862d145b928e9b4b975.

The original NFSv4.0 AUTH_SYS SAVEFH reproducer returned WRONGSEC for
READ and WRITE against the krb5p export. The destination was unchanged,
and none of 108 candidate stateids returned the protected data. The
v4.2 saved-source COPY and CLONE tests returned ACCESS on the denied
source and left the destination unchanged. Authorized local COPY and
CLONE, inter-server COPY with COPY_NOTIFY and OFFLOAD_STATUS, and the
asynchronous negative cases behaved as expected.

I also reran the v6 regression matrix on v7: SAVEFH and RESTOREFH
boundaries, error ordering, LOOKUP and LOOKUPP, OPEN by name and by
filehandle, SECINFO and SECINFO_NO_NAME, READDIR with rdattr_error,
LINK and RENAME, NFSv2/v3 CREATE, v4 OPEN UNCHECKED, and export policy
changes from sec=sys to krb5p. The GSS cases covered krb5, krb5i,
and krb5p on v4.0, v4.1, and v4.2. These checks passed.

For patches 4 and 5, I ran the GSS cases against the changed SECINFO
and PUTFH paths. PUTFH deferred the flavor error where required;
SECINFO returned the available flavors; a following operation did not
use a stale current filehandle. I repeated the root and child-export
checks over plaintext, TLS, and mTLS on v4.0, v4.1, and v4.2, and ran
krb5p over TLS 1.3 on v4.1 and v4.2. The permitted requests succeeded
and the denied requests returned the expected errors. No KASAN report
or kernel oops appeared in the guest log.

The W=1 NFSD object builds passed with NFSD_V4_2_INTER_SSC enabled and
disabled. The need_wrongsec_check() decision harness passed 259/259
cases.

Tested-by: VeroFess <coderzeng@gmail.com>

VeroFess