NFSD: Map flex file layout IDs through the request's user namespace
Subsystems (2)
Chuck Lever
maintainerPATCH 1/1 NFSD: Map flex file layout IDs through the request's user namespace
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.
--- fs/nfsd/flexfilelayout.c | 20 ++++++++++++-------- fs/nfsd/flexfilelayoutxdr.c | 4 ++-- fs/nfsd/flexfilelayoutxdr.h | 5 +++-- 3 files changed, 17 insertions(+), 12 deletions(-)
fs/nfsd/flexfilelayout.c +12 -8
@@ -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)
fs/nfsd/flexfilelayoutxdr.c +2 -2
@@ -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
fs/nfsd/flexfilelayoutxdr.h +3 -2
@@ -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;
Signature · 1 line
2.54.0
Quoted 112 lines · Chuck Lever
nfsd4_ff_proc_layoutget()andnfsd4_ff_encode_layoutget()translate the file's owner and group withinit_user_ns, but every other identity nfsd places on the wire goes throughnfsd_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 theAUTH_SYScredential it presents to the data server, andsvcauth_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 theIOMODE_READarm 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 thesvc_rqst, and carry the wire values in structpnfs_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 <[email protected]>Signed-off-by Chuck Lever <[email protected]>---
fs/nfsd/flexfilelayout.c | 20 ++++++++++++--------fs/nfsd/flexfilelayoutxdr.c| 4 ++--fs/nfsd/flexfilelayoutxdr.h| 5 +++--3 files changed, 17 insertions(+), 12 deletions(-)fs/nfsd/flexfilelayout.c +0
-0@@ -15,6 +15,7 @@
#include "nfserr.h"
#include "flexfilelayoutxdr.h"
+#include "auth.h"
#include "pnfs.h"
#include "vfs.h"fs/nfsd/flexfilelayout.c +1
-1@@ -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;
structpnfs_ff_layout*fl;fs/nfsd/flexfilelayout.c +0
-0@@ -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;fs/nfsd/flexfilelayout.c +10
-7-/* 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)fs/nfsd/flexfilelayoutxdr.c +0
-0@@ -33,8 +33,8 @@ nfsd4_ff_encode_layoutget(struct xdr_stream *xdr,
fh_len= 4 +xdr_align_size(fl->fh.size);fs/nfsd/flexfilelayoutxdr.c +2
-2- 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_hintfs/nfsd/flexfilelayoutxdr.h +3
-3@@ -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; --
Quoted 1 line
2.54.0