mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window
@ 2026-11-04 11:56 orkunsari via B4 Relay
  2026-11-04 11:56 ` [PATCH 1/2] nfsd: unlock the locked parent after crossing a mount in CREATE orkunsari via B4 Relay
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: orkunsari via B4 Relay @ 2026-11-04 11:56 UTC (permalink / raw)
  To: Chuck Lever, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
	Tom Talpey
  Cc: linux-nfs, linux-kernel, orkunsari

Two fixes for code merged for v7.4-rc1; released kernels are not
affected.

Patch 1 fixes the NFSv2/v3 CREATE paths, which pass the dentry returned
by nfsd_cross_mnt() to end_creating() and so unlock the wrong directory
when the target file has something mounted on it.

Patch 2 makes nfsd_cross_mnt() check access to the export being entered
rather than the one being left, and restores the check after ".." in
nfsd_lookup(). SECINFO keeps its previous behaviour and does not return
NFS4ERR_WRONGSEC.

Both were found by AI-assisted code review (Claude Code). The fixes are
build-tested with W=1 only; no reproducer was run.

---
orkunsari (2):
      nfsd: unlock the locked parent after crossing a mount in CREATE
      nfsd: check access to the export being entered when crossing mounts

 fs/nfsd/nfs3proc.c | 14 +++++++++++---
 fs/nfsd/nfsproc.c  | 14 +++++++++++---
 fs/nfsd/vfs.c      | 46 +++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 61 insertions(+), 13 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261104-nfsd-cross-mnt-1f84d7972869

Best regards,
--  
orkunsari <sariorkunemin@gmail.com>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] nfsd: unlock the locked parent after crossing a mount in CREATE
  2026-11-04 11:56 [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window orkunsari via B4 Relay
@ 2026-11-04 11:56 ` orkunsari via B4 Relay
  2026-11-04 11:56 ` [PATCH 2/2] nfsd: check access to the export being entered when crossing mounts orkunsari via B4 Relay
  2026-11-04 19:48 ` [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window Chuck Lever
  2 siblings, 0 replies; 5+ messages in thread
From: orkunsari via B4 Relay @ 2026-11-04 11:56 UTC (permalink / raw)
  To: Chuck Lever, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
	Tom Talpey
  Cc: linux-nfs, linux-kernel, orkunsari

From: orkunsari <sariorkunemin@gmail.com>

Since commit f58a48389c14 ("nfsd: correctly handle CREATE of mounted-on
files"), nfsd_proc_create() and nfsd3_create_file() call
nfsd_cross_mnt() on the dentry returned by start_creating().  When the
name refers to a file that has something mounted on it,
nfsd_cross_mnt() replaces that dentry with the root of the mounted
filesystem and drops its reference to the original one.

The replacement is later passed to end_creating(), which unlocks the
d_parent of the dentry it is given.  That is the parent of the mount
source, not the directory start_creating() locked, so the directory
being created in is left locked and another directory's i_rwsem is
released without being held.

Do the crossing on a separate reference that is only used for
fh_compose(), so that the dentry returned by start_creating() is the
one passed to end_creating().  Only positive regular files are crossed,
so the later checks on the child dentry give the same result.

This was found by AI-assisted code review (Claude Code).  The fix has
been build-tested with W=1 only; no reproducer was run.

Fixes: f58a48389c14 ("nfsd: correctly handle CREATE of mounted-on files")
Signed-off-by: orkunsari <sariorkunemin@gmail.com>
---
 fs/nfsd/nfs3proc.c | 14 +++++++++++---
 fs/nfsd/nfsproc.c  | 14 +++++++++++---
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 60cd01b6a3..0711e93c99 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -334,7 +334,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		  struct svc_fh *resfhp, struct nfsd3_createargs *argp)
 {
 	struct iattr *iap = &argp->attrs;
-	struct dentry *parent, *child;
+	struct dentry *parent, *child, *mnt_child;
 	struct nfsd_attrs attrs = {
 		.na_iattr	= iap,
 	};
@@ -378,21 +378,29 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	}
 
 	exp = exp_get(fhp->fh_export);
+	/*
+	 * Cross on a separate reference: child must remain the dentry
+	 * start_creating() returned so that end_creating() unlocks the
+	 * right directory.
+	 */
+	mnt_child = dget(child);
 	if (argp->createmode == NFS3_CREATE_UNCHECKED) {
 		/*
 		 * If name is already in dcache we need to check for mountpoints
 		 */
 		if (d_is_reg(child) &&
 		    unlikely(nfsd_mountpoint(child, exp))) {
-			status = nfsd_cross_mnt(rqstp, &child, &exp);
+			status = nfsd_cross_mnt(rqstp, &mnt_child, &exp);
 			if (status != nfs_ok) {
+				dput(mnt_child);
 				exp_put(exp);
 				goto out;
 			}
 		}
 	}
 
-	status = fh_compose(resfhp, exp, child, fhp);
+	status = fh_compose(resfhp, exp, mnt_child, fhp);
+	dput(mnt_child);
 	exp_put(exp);
 	if (status != nfs_ok)
 		goto out;
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 1f74008bc0..7da653bc0e 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -682,7 +682,7 @@ nfsd_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		.na_iattr	= attr,
 	};
 	struct svc_export *exp;
-	struct dentry *dchild;
+	struct dentry *dchild, *dmnt;
 	struct inode *inode;
 	int type, mode;
 	__be32 status;
@@ -709,16 +709,24 @@ nfsd_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	 * If name exists we need to check for mountpoints
 	 */
 	exp = exp_get(fhp->fh_export);
+	/*
+	 * Cross on a separate reference: dchild must remain the dentry
+	 * start_creating() returned so that end_creating() unlocks the
+	 * right directory.
+	 */
+	dmnt = dget(dchild);
 	if (d_is_reg(dchild) &&
 	    unlikely(nfsd_mountpoint(dchild, exp))) {
-		status = nfsd_cross_mnt(rqstp, &dchild, &exp);
+		status = nfsd_cross_mnt(rqstp, &dmnt, &exp);
 		if (status != nfs_ok) {
+			dput(dmnt);
 			exp_put(exp);
 			goto out;
 		}
 	}
 
-	status = fh_compose(resfhp, exp, dchild, fhp);
+	status = fh_compose(resfhp, exp, dmnt, fhp);
+	dput(dmnt);
 	exp_put(exp);
 	if (!status && d_really_is_negative(dchild))
 		status = nfserr_noent;

-- 
2.56.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] nfsd: check access to the export being entered when crossing mounts
  2026-11-04 11:56 [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window orkunsari via B4 Relay
  2026-11-04 11:56 ` [PATCH 1/2] nfsd: unlock the locked parent after crossing a mount in CREATE orkunsari via B4 Relay
@ 2026-11-04 11:56 ` orkunsari via B4 Relay
  2026-11-04 18:52   ` Chuck Lever
  2026-11-04 19:48 ` [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window Chuck Lever
  2 siblings, 1 reply; 5+ messages in thread
From: orkunsari via B4 Relay @ 2026-11-04 11:56 UTC (permalink / raw)
  To: Chuck Lever, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
	Tom Talpey
  Cc: linux-nfs, linux-kernel, orkunsari

From: orkunsari <sariorkunemin@gmail.com>

Commit f0d6ad5a2f67 ("nfsd: move check_nfsd_access() call into
nfsd_cross_mnt()") moved the check_nfsd_access() calls from
nfsd_lookup() and the NFSv4 READDIR encoder into nfsd_cross_mnt().  The
moved call checks @exp, the export being left, which the caller has
already verified, rather than @exp2, the export being entered.  The
sec= and xprtsec= settings of a crossmnt/nohide child export are
therefore no longer applied when LOOKUP or READDIR returns its root
filehandle and attributes.  Later operations on that filehandle are
still checked by fh_verify().

The same commit also removed the check that nfsd_lookup() applied after
a ".." lookup steps up into a parent export via nfsd_lookup_parent().

Simply checking @exp2 instead is not enough: nfsd_lookup_dentry() also
serves SECINFO, which must not fail with NFS4ERR_WRONGSEC (RFC 8881,
section 2.6.3.1.1.5) and did not check the target export before.

Split the crossing into __nfsd_cross_mnt(), which checks @exp2 before
switching to it only when asked to.  nfsd_cross_mnt() keeps checking,
for READDIR and the CREATE paths, while nfsd_lookup_dentry() does not.
nfsd_lookup() checks the resulting export whenever it differs from the
directory's, which covers both mount crossings and "..", as it did
before.

This was found by AI-assisted code review (Claude Code).  The fix has
been build-tested with W=1 only; no reproducer was run.

Fixes: f0d6ad5a2f67 ("nfsd: move check_nfsd_access() call into nfsd_cross_mnt()")
Signed-off-by: orkunsari <sariorkunemin@gmail.com>
---
 fs/nfsd/vfs.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 56c49b5bc5..2df2df5aaf 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -120,14 +120,16 @@ 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.  If @check_access is set, the
+ * export being entered must also pass check_nfsd_access().
+ * nfsd_lookup_dentry() passes false, as it also serves SECINFO; its
+ * other users check the resulting export themselves.
  * 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)
+static __be32
+__nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
+		 struct svc_export **expp, bool check_access)
 {
 	struct svc_export *exp = *expp, *exp2 = NULL;
 	struct dentry *dentry = *dpp;
@@ -165,7 +167,8 @@ 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);
+		if (check_access)
+			nfserr = check_nfsd_access(exp2, rqstp);
 		if (nfserr == nfs_ok) {
 			/* successfully crossed mount point */
 			/*
@@ -189,6 +192,20 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
 	return nfserrno(err);
 }
 
+/*
+ * Called from encode_dirent and the CREATE procedures. Check if we have
+ * crossed a mount point, and if so whether the client may access the
+ * export being entered.
+ * 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)
+{
+	return __nfsd_cross_mnt(rqstp, dpp, expp, true);
+}
+
 static void follow_to_parent(struct path *path)
 {
 	struct dentry *dp;
@@ -284,7 +301,12 @@ 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);
+			/*
+			 * Access to the new export is checked by the caller
+			 * where needed; SECINFO must not fail with WRONGSEC.
+			 */
+			__be32 nfserr = __nfsd_cross_mnt(rqstp, &dentry, &exp,
+							 false);
 
 			if (nfserr) {
 				dput(dentry);
@@ -336,6 +358,15 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
 	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
 	if (err)
 		return err;
+	/*
+	 * The lookup may have crossed into another export, either down
+	 * through a mount point or up through "..".
+	 */
+	if (exp != fhp->fh_export) {
+		err = check_nfsd_access(exp, rqstp);
+		if (err)
+			goto out;
+	}
 	/*
 	 * Note: we compose the file handle now, but as the
 	 * dentry may be negative, it may need to be updated.
@@ -344,6 +375,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
 	if (!err && d_really_is_negative(dentry))
 		err = nfserr_noent;
 
+out:
 	dput(dentry);
 	exp_put(exp);
 	return err;

-- 
2.56.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] nfsd: check access to the export being entered when crossing mounts
  2026-11-04 11:56 ` [PATCH 2/2] nfsd: check access to the export being entered when crossing mounts orkunsari via B4 Relay
@ 2026-11-04 18:52   ` Chuck Lever
  0 siblings, 0 replies; 5+ messages in thread
From: Chuck Lever @ 2026-11-04 18:52 UTC (permalink / raw)
  To: sariorkunemin, Jeff Layton, NeilBrown, Olga Kornievskaia,
	Dai Ngo, Tom Talpey
  Cc: linux-nfs, linux-kernel



On Wed, Nov 4, 2026, at 6:56 AM, orkunsari via B4 Relay wrote:
> From: orkunsari <sariorkunemin@gmail.com>
>
> Commit f0d6ad5a2f67 ("nfsd: move check_nfsd_access() call into
> nfsd_cross_mnt()") moved the check_nfsd_access() calls from
> nfsd_lookup() and the NFSv4 READDIR encoder into nfsd_cross_mnt().  The
> moved call checks @exp, the export being left, which the caller has
> already verified, rather than @exp2, the export being entered.  The
> sec= and xprtsec= settings of a crossmnt/nohide child export are
> therefore no longer applied when LOOKUP or READDIR returns its root
> filehandle and attributes.  Later operations on that filehandle are
> still checked by fh_verify().
>
> The same commit also removed the check that nfsd_lookup() applied after
> a ".." lookup steps up into a parent export via nfsd_lookup_parent().
>
> Simply checking @exp2 instead is not enough: nfsd_lookup_dentry() also
> serves SECINFO, which must not fail with NFS4ERR_WRONGSEC (RFC 8881,
> section 2.6.3.1.1.5) and did not check the target export before.
>
> Split the crossing into __nfsd_cross_mnt(), which checks @exp2 before
> switching to it only when asked to.  nfsd_cross_mnt() keeps checking,
> for READDIR and the CREATE paths, while nfsd_lookup_dentry() does not.
> nfsd_lookup() checks the resulting export whenever it differs from the
> directory's, which covers both mount crossings and "..", as it did
> before.
>
> This was found by AI-assisted code review (Claude Code).  The fix has
> been build-tested with W=1 only; no reproducer was run.
>
> Fixes: f0d6ad5a2f67 ("nfsd: move check_nfsd_access() call into 
> nfsd_cross_mnt()")
> Signed-off-by: orkunsari <sariorkunemin@gmail.com>

This issue is very likely addressed by patches I posted earlier this
week in response to a similar vulnerability report.

  - "NFSD: Check the entered export on a mount crossing" checks the
    entered export inside nfsd_cross_mnt and in nfsd_lookup_parent,
    which covers the ".." case that 2/2 also raises.
  - "NFSD: Let SECINFO query an export that excludes the request's
    flavor" handles the SECINFO concern the patch solves with its
    __nfsd_cross_mnt split.
  - Two follow-ups (secure-port policy, applying the entered export's
    credentials after LOOKUP and RESTOREFH) rework the same region
    further.

Let's set 2/2 aside for now, because the series I posted seems more
thorough.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window
  2026-11-04 11:56 [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window orkunsari via B4 Relay
  2026-11-04 11:56 ` [PATCH 1/2] nfsd: unlock the locked parent after crossing a mount in CREATE orkunsari via B4 Relay
  2026-11-04 11:56 ` [PATCH 2/2] nfsd: check access to the export being entered when crossing mounts orkunsari via B4 Relay
@ 2026-11-04 19:48 ` Chuck Lever
  2 siblings, 0 replies; 5+ messages in thread
From: Chuck Lever @ 2026-11-04 19:48 UTC (permalink / raw)
  To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	orkunsari
  Cc: linux-nfs, linux-kernel

On Wed, 04 Nov 2026 14:56:14 +0300, orkunsari wrote:
> Two fixes for code merged for v7.4-rc1; released kernels are not
> affected.
> 
> Patch 1 fixes the NFSv2/v3 CREATE paths, which pass the dentry returned
> by nfsd_cross_mnt() to end_creating() and so unlock the wrong directory
> when the target file has something mounted on it.
> 
> [...]

Applied to nfsd-testing, thanks!

[1/2] nfsd: unlock the locked parent after crossing a mount in CREATE
      commit: 0e0796416999f96e4192a25ac589f57bb497176f

--
Chuck Lever


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-04 19:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-04 11:56 [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window orkunsari via B4 Relay
2026-11-04 11:56 ` [PATCH 1/2] nfsd: unlock the locked parent after crossing a mount in CREATE orkunsari via B4 Relay
2026-11-04 11:56 ` [PATCH 2/2] nfsd: check access to the export being entered when crossing mounts orkunsari via B4 Relay
2026-11-04 18:52   ` Chuck Lever
2026-11-04 19:48 ` [PATCH 0/2] nfsd: fix two mount-crossing regressions from the v7.4 merge window Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®