Just to provide closure on this, I just committed a patch to main (that will be MFC'd in 2 weeks) that sets MNT_EXPORTED when the "export" option is specified to nmount(2).
This restores the "only root can do exports" semantics that appear to have been the case pre-r158857. Thanks everyone for your comments, rick On Wed, Dec 14, 2022 at 9:15 AM Tomoaki AOKI <junch...@dec.sakura.ne.jp> wrote: > Tracking the commits, it was originally introduced to > sys/kern/vfs_syscalls.c at r22521 [1][2] (Mon Feb 10 02:22:35 1997 by > dyson, submitted by h...@freebsd.org) and later centralized into > sys/kern/vfs_mount.c at r99264 [2]. > > But according to the comment above the codes, maybe it would be > intended to block userland programs or ports FS modules setting > MNT_EXPORTED. > > If I'm not mis-understanding, it can be the case when > *vfs.usermount sysctl is non-zero, > *underlying FS (to be exported) allows it, and > *non-root user tries to mount the FS via NFS. > > > [1] > > https://svnweb.freebsd.org/base/head/sys/kern/vfs_syscalls.c?revision=22521&view=markup&pathrev=99264 > > [2] > > https://svnweb.freebsd.org/base/head/sys/kern/vfs_syscalls.c?r1=22520&r2=22521&pathrev=99264&; > > [3] > > https://cgit.freebsd.org/src/commit/sys/kern/vfs_mount.c?id=2b4edb69f1ef62fc38b02ac22b0a3ac09e43fa77 > > > On Tue, 13 Dec 2022 14:19:39 -0800 > Rick Macklem <rick.mack...@gmail.com> wrote: > > > Hi, > > > > While working on getting mountd/nfsd to run in a vnet > > prison, I came across the following lines near the > > beginning of vfs_domount() in sys/kern/vfs_mount.c: > > > > if (fsflags & MNT_EXPORTED) { > > error = priv_check(td, PRIV_VFS_MOUNT_EXPORTED); > > if (error) > > return (error); > > } > > > > #1 - Since MNT_EXPORTED is never set in fsflags, this code never > > gets executed. > > --> I am asking what to do with the above code, since that > > changes for the patch that allows mountd to run in a vnet > > prison. > > #2 - priv_check(td, PRIV_VFS_MOUNT_EXPORTED) always returns 0 > > because nothing in sys/kern/kern_priv.c checks > > PRIV_VFS_MOUNT_EXPORTED. > > > > I don't know what the original author's thinking was w.r.t. this. > > Setting exports already checks that the mount operation can be > > done by the requestor. > > > > So, what do you think should be done with the above code snippet? > > - Consider it cruft and delete it. > > - Try and figure out what PRIV_VFS_MOUNT_EXPORTED should check? > > - Leave it as is. After the patch that allows mountd to run in > > a vnet prison, MNT_EXPORTED will be set in fsflags, but the > > priv_check() call will just return 0. (A little overhead, > > but otherwise no semantics change.) > > > > rick > > > -- > Tomoaki AOKI <junch...@dec.sakura.ne.jp> > >
