Re: What to do about a few lines in vfs_domount() never executed?

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 <junchoon@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 hsu@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.macklem@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    <junchoon@dec.sakura.ne.jp>
>
>