All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NFSD: Map flex file layout IDs through the request's user namespace
@ 2026-07-20 16:37 Chuck Lever
  2026-07-21 11:25 ` Jeff Layton
  0 siblings, 1 reply; 2+ messages in thread
From: Chuck Lever @ 2026-07-20 16:37 UTC (permalink / raw)
  To: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, sashiko-bot

nfsd4_ff_proc_layoutget() and nfsd4_ff_encode_layoutget() translate
the file's owner and group with init_user_ns, but every other identity
nfsd places on the wire goes through nfsd_user_namespace(). When the
transport carries a credential from a non-initial user namespace, the
flex file layout reports host-global IDs. The client copies those IDs
into the AUTH_SYS credential it presents to the data server, and
svcauth_unix_accept() resolves that credential in the transport's
namespace, so data server I/O runs under an identity unrelated to the
file's owner.

Switching to the request's namespace introduces a second hazard.
from_kuid() returns (uid_t)-1 when the target namespace has no
mapping for the owner, and the IOMODE_READ arm adds one to that
result to derive an identity for which the data server denies
writes. The addition would wrap to zero, handing the client uid 0
instead of an identity distinct from the owner.

Translate both IDs in nfsd4_ff_proc_layoutget(), which has the
svc_rqst, and carry the wire values in struct pnfs_ff_layout.
from_kuid_munged() substitutes overflowuid for an unmapped owner and
thus never returns (uid_t)-1, so the increment cannot wrap to zero.

Fixes: 9b9960a0ca47 ("nfsd: Add a super simple flex file server")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260720141442.783935-1-cel@kernel.org?part=3
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/flexfilelayout.c    | 20 ++++++++++++--------
 fs/nfsd/flexfilelayoutxdr.c |  4 ++--
 fs/nfsd/flexfilelayoutxdr.h |  5 +++--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/flexfilelayout.c b/fs/nfsd/flexfilelayout.c
index 9f532418cac8..c6e6b9106a83 100644
--- a/fs/nfsd/flexfilelayout.c
+++ b/fs/nfsd/flexfilelayout.c
@@ -15,6 +15,7 @@
 
 #include "nfserr.h"
 #include "flexfilelayoutxdr.h"
+#include "auth.h"
 #include "pnfs.h"
 #include "vfs.h"
 
@@ -24,10 +25,10 @@ static __be32
 nfsd4_ff_proc_layoutget(struct svc_rqst *rqstp, struct inode *inode,
 		const struct svc_fh *fhp, struct nfsd4_layoutget *args)
 {
+	struct user_namespace *userns = nfsd_user_namespace(rqstp);
 	struct nfsd4_layout_seg *seg = &args->lg_seg;
 	u32 device_generation = 0;
 	int error;
-	uid_t u;
 
 	struct pnfs_ff_layout *fl;
 
@@ -50,13 +51,16 @@ nfsd4_ff_proc_layoutget(struct svc_rqst *rqstp, struct inode *inode,
 	fl->flags = FF_FLAGS_NO_LAYOUTCOMMIT | FF_FLAGS_NO_IO_THRU_MDS |
 		    FF_FLAGS_NO_READ_IO;
 
-	/* Do not allow a IOMODE_READ segment to have write pemissions */
-	if (seg->iomode == IOMODE_READ) {
-		u = from_kuid(&init_user_ns, inode->i_uid) + 1;
-		fl->uid = make_kuid(&init_user_ns, u);
-	} else
-		fl->uid = inode->i_uid;
-	fl->gid = inode->i_gid;
+	fl->uid = from_kuid_munged(userns, inode->i_uid);
+	fl->gid = from_kgid_munged(userns, inode->i_gid);
+
+	/*
+	 * Do not allow an IOMODE_READ segment to have write permissions.
+	 * The group is left intact so group-readable files stay readable;
+	 * nfsd_setuser() squashes an unmapped uid to the export's anon ID.
+	 */
+	if (seg->iomode == IOMODE_READ)
+		fl->uid++;
 
 	error = nfsd4_set_deviceid(&fl->deviceid, fhp, device_generation);
 	if (error)
diff --git a/fs/nfsd/flexfilelayoutxdr.c b/fs/nfsd/flexfilelayoutxdr.c
index 97d8a28dd3a0..c12bb7c371b0 100644
--- a/fs/nfsd/flexfilelayoutxdr.c
+++ b/fs/nfsd/flexfilelayoutxdr.c
@@ -33,8 +33,8 @@ nfsd4_ff_encode_layoutget(struct xdr_stream *xdr,
 
 	fh_len = 4 + xdr_align_size(fl->fh.size);
 
-	uid.len = sprintf(uid.buf, "%u", from_kuid(&init_user_ns, fl->uid));
-	gid.len = sprintf(gid.buf, "%u", from_kgid(&init_user_ns, fl->gid));
+	uid.len = sprintf(uid.buf, "%u", fl->uid);
+	gid.len = sprintf(gid.buf, "%u", fl->gid);
 
 	/* data server entry: deviceid + efficiency + stateid + fh list +
 	 * user + group + flags + stats_collect_hint
diff --git a/fs/nfsd/flexfilelayoutxdr.h b/fs/nfsd/flexfilelayoutxdr.h
index 6d5a1066a903..3e1876d49db2 100644
--- a/fs/nfsd/flexfilelayoutxdr.h
+++ b/fs/nfsd/flexfilelayoutxdr.h
@@ -35,8 +35,9 @@ struct pnfs_ff_device_addr {
 struct pnfs_ff_layout {
 	u32				flags;
 	u32				stats_collect_hint;
-	kuid_t				uid;
-	kgid_t				gid;
+	/* Values to encode; nfsd4_ff_proc_layoutget() has mapped these */
+	u32				uid;
+	u32				gid;
 	struct nfsd4_deviceid		deviceid;
 	stateid_t			stateid;
 	struct nfs_fh			fh;
-- 
2.54.0


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

* Re: [PATCH] NFSD: Map flex file layout IDs through the request's user namespace
  2026-07-20 16:37 [PATCH] NFSD: Map flex file layout IDs through the request's user namespace Chuck Lever
@ 2026-07-21 11:25 ` Jeff Layton
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff Layton @ 2026-07-21 11:25 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, sashiko-bot

On Mon, 2026-07-20 at 12:37 -0400, Chuck Lever wrote:
> nfsd4_ff_proc_layoutget() and nfsd4_ff_encode_layoutget() translate
> the file's owner and group with init_user_ns, but every other identity
> nfsd places on the wire goes through nfsd_user_namespace(). When the
> transport carries a credential from a non-initial user namespace, the
> flex file layout reports host-global IDs. The client copies those IDs
> into the AUTH_SYS credential it presents to the data server, and
> svcauth_unix_accept() resolves that credential in the transport's
> namespace, so data server I/O runs under an identity unrelated to the
> file's owner.
> 
> Switching to the request's namespace introduces a second hazard.
> from_kuid() returns (uid_t)-1 when the target namespace has no
> mapping for the owner, and the IOMODE_READ arm adds one to that
> result to derive an identity for which the data server denies
> writes. The addition would wrap to zero, handing the client uid 0
> instead of an identity distinct from the owner.
> 
> Translate both IDs in nfsd4_ff_proc_layoutget(), which has the
> svc_rqst, and carry the wire values in struct pnfs_ff_layout.
> from_kuid_munged() substitutes overflowuid for an unmapped owner and
> thus never returns (uid_t)-1, so the increment cannot wrap to zero.
> 
> Fixes: 9b9960a0ca47 ("nfsd: Add a super simple flex file server")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260720141442.783935-1-cel@kernel.org?part=3
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
>  fs/nfsd/flexfilelayout.c    | 20 ++++++++++++--------
>  fs/nfsd/flexfilelayoutxdr.c |  4 ++--
>  fs/nfsd/flexfilelayoutxdr.h |  5 +++--
>  3 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/nfsd/flexfilelayout.c b/fs/nfsd/flexfilelayout.c
> index 9f532418cac8..c6e6b9106a83 100644
> --- a/fs/nfsd/flexfilelayout.c
> +++ b/fs/nfsd/flexfilelayout.c
> @@ -15,6 +15,7 @@
>  
>  #include "nfserr.h"
>  #include "flexfilelayoutxdr.h"
> +#include "auth.h"
>  #include "pnfs.h"
>  #include "vfs.h"
>  
> @@ -24,10 +25,10 @@ static __be32
>  nfsd4_ff_proc_layoutget(struct svc_rqst *rqstp, struct inode *inode,
>  		const struct svc_fh *fhp, struct nfsd4_layoutget *args)
>  {
> +	struct user_namespace *userns = nfsd_user_namespace(rqstp);
>  	struct nfsd4_layout_seg *seg = &args->lg_seg;
>  	u32 device_generation = 0;
>  	int error;
> -	uid_t u;
>  
>  	struct pnfs_ff_layout *fl;
>  
> @@ -50,13 +51,16 @@ nfsd4_ff_proc_layoutget(struct svc_rqst *rqstp, struct inode *inode,
>  	fl->flags = FF_FLAGS_NO_LAYOUTCOMMIT | FF_FLAGS_NO_IO_THRU_MDS |
>  		    FF_FLAGS_NO_READ_IO;
>  
> -	/* Do not allow a IOMODE_READ segment to have write pemissions */
> -	if (seg->iomode == IOMODE_READ) {
> -		u = from_kuid(&init_user_ns, inode->i_uid) + 1;
> -		fl->uid = make_kuid(&init_user_ns, u);
> -	} else
> -		fl->uid = inode->i_uid;
> -	fl->gid = inode->i_gid;
> +	fl->uid = from_kuid_munged(userns, inode->i_uid);
> +	fl->gid = from_kgid_munged(userns, inode->i_gid);
> +
> +	/*
> +	 * Do not allow an IOMODE_READ segment to have write permissions.
> +	 * The group is left intact so group-readable files stay readable;
> +	 * nfsd_setuser() squashes an unmapped uid to the export's anon ID.
> +	 */
> +	if (seg->iomode == IOMODE_READ)
> +		fl->uid++;
>  
>  	error = nfsd4_set_deviceid(&fl->deviceid, fhp, device_generation);
>  	if (error)
> diff --git a/fs/nfsd/flexfilelayoutxdr.c b/fs/nfsd/flexfilelayoutxdr.c
> index 97d8a28dd3a0..c12bb7c371b0 100644
> --- a/fs/nfsd/flexfilelayoutxdr.c
> +++ b/fs/nfsd/flexfilelayoutxdr.c
> @@ -33,8 +33,8 @@ nfsd4_ff_encode_layoutget(struct xdr_stream *xdr,
>  
>  	fh_len = 4 + xdr_align_size(fl->fh.size);
>  
> -	uid.len = sprintf(uid.buf, "%u", from_kuid(&init_user_ns, fl->uid));
> -	gid.len = sprintf(gid.buf, "%u", from_kgid(&init_user_ns, fl->gid));
> +	uid.len = sprintf(uid.buf, "%u", fl->uid);
> +	gid.len = sprintf(gid.buf, "%u", fl->gid);
>  
>  	/* data server entry: deviceid + efficiency + stateid + fh list +
>  	 * user + group + flags + stats_collect_hint
> diff --git a/fs/nfsd/flexfilelayoutxdr.h b/fs/nfsd/flexfilelayoutxdr.h
> index 6d5a1066a903..3e1876d49db2 100644
> --- a/fs/nfsd/flexfilelayoutxdr.h
> +++ b/fs/nfsd/flexfilelayoutxdr.h
> @@ -35,8 +35,9 @@ struct pnfs_ff_device_addr {
>  struct pnfs_ff_layout {
>  	u32				flags;
>  	u32				stats_collect_hint;
> -	kuid_t				uid;
> -	kgid_t				gid;
> +	/* Values to encode; nfsd4_ff_proc_layoutget() has mapped these */
> +	u32				uid;
> +	u32				gid;
>  	struct nfsd4_deviceid		deviceid;
>  	stateid_t			stateid;
>  	struct nfs_fh			fh;
> --
> 2.54.0

Reviewed-by: Jeff Layton <jlayton@kernel.org>

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

end of thread, other threads:[~2026-07-21 11:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 16:37 [PATCH] NFSD: Map flex file layout IDs through the request's user namespace Chuck Lever
2026-07-21 11:25 ` Jeff Layton

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.