Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 2 Jul 2019 08:34:32 +0200
From:      Conrad Meyer <cem@freebsd.org>
To:        Kirk McKusick <mckusick@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r349589 - in head: sbin/mount sys/sys sys/ufs/ffs
Message-ID:  <CAG6CVpWgbnRyC95yY4orMT69dzS=3AazgPE_ZiROYrDHKcaHgQ@mail.gmail.com>
In-Reply-To: <201907012322.x61NMRGS078268@repo.freebsd.org>
References:  <201907012322.x61NMRGS078268@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi,

Maybe the sense of the flag should be reversed? Ie, add a =E2=80=9Ctrusted=
=E2=80=9D flag
and default to untrusted.

I have two reasons in mind. The first is that a new default-off option is
easy to forget, and a missed security feature may be worse than a missed
mount-time performance enhancement.

The second is just the basic idea of preferring  to avoid double negatives
in flag names.

Thanks,
Conrad

On Tue, Jul 2, 2019 at 04:21 Kirk McKusick <mckusick@freebsd.org> wrote:

> Author: mckusick
> Date: Mon Jul  1 23:22:26 2019
> New Revision: 349589
> URL: https://svnweb.freebsd.org/changeset/base/349589
>
> Log:
>   Add a new "untrusted" option to the mount command. Its purpose
>   is to notify the kernel that the file system is untrusted and it
>   should use more extensive checks on the file-system's metadata
>   before using it. This option is intended to be used when mounting
>   file systems from untrusted media such as USB memory sticks or other
>   externally-provided media.
>
>   It will initially be used by the UFS/FFS file system, but should
>   likely be expanded to be used by other file systems that may appear
>   on external media like msdosfs, exfat, and ext2fs.
>
>   Reviewed by:  kib
>   Sponsored by: Netflix
>   Differential Revision: https://reviews.freebsd.org/D20786
>
> Modified:
>   head/sbin/mount/mntopts.h
>   head/sbin/mount/mount.8
>   head/sbin/mount/mount.c
>   head/sys/sys/mount.h
>   head/sys/ufs/ffs/ffs_vfsops.c
>
> Modified: head/sbin/mount/mntopts.h
>
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sbin/mount/mntopts.h   Mon Jul  1 22:11:56 2019        (r349588)
> +++ head/sbin/mount/mntopts.h   Mon Jul  1 23:22:26 2019        (r349589)
> @@ -58,6 +58,7 @@ struct mntopt {
>  #define MOPT_ACLS              { "acls",       0, MNT_ACLS, 0 }
>  #define MOPT_NFS4ACLS          { "nfsv4acls",  0, MNT_NFS4ACLS, 0 }
>  #define MOPT_AUTOMOUNTED       { "automounted",0, MNT_AUTOMOUNTED, 0 }
> +#define MOPT_UNTRUSTED         { "untrusted",  0, MNT_UNTRUSTED, 0 }
>
>  /* Control flags. */
>  #define MOPT_FORCE             { "force",      0, MNT_FORCE, 0 }
> @@ -93,7 +94,8 @@ struct mntopt {
>         MOPT_MULTILABEL,                                                \
>         MOPT_ACLS,                                                      \
>         MOPT_NFS4ACLS,                                                  \
> -       MOPT_AUTOMOUNTED
> +       MOPT_AUTOMOUNTED,                                               \
> +       MOPT_UNTRUSTED
>
>  void getmntopts(const char *, const struct mntopt *, int *, int *);
>  void rmslashes(char *, char *);
>
> Modified: head/sbin/mount/mount.8
>
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sbin/mount/mount.8     Mon Jul  1 22:11:56 2019        (r349588)
> +++ head/sbin/mount/mount.8     Mon Jul  1 23:22:26 2019        (r349589)
> @@ -355,6 +355,12 @@ Lookups will be done in the mounted file system firs=
t.
>  If those operations fail due to a non-existent file the underlying
>  directory is then accessed.
>  All creates are done in the mounted file system.
> +.It Cm untrusted
> +The file system is untrusted and the kernel should use more
> +extensive checks on the file-system's metadata before using it.
> +This option is intended to be used when mounting file systems
> +from untrusted media such as USB memory sticks or other
> +externally-provided media.
>  .El
>  .Pp
>  Any additional options specific to a file system type that is not
>
> Modified: head/sbin/mount/mount.c
>
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sbin/mount/mount.c     Mon Jul  1 22:11:56 2019        (r349588)
> +++ head/sbin/mount/mount.c     Mon Jul  1 23:22:26 2019        (r349589)
> @@ -118,6 +118,7 @@ static struct opt {
>         { MNT_GJOURNAL,         "gjournal" },
>         { MNT_AUTOMOUNTED,      "automounted" },
>         { MNT_VERIFIED,         "verified" },
> +       { MNT_UNTRUSTED,        "untrusted" },
>         { 0, NULL }
>  };
>
> @@ -972,6 +973,7 @@ flags2opts(int flags)
>         if (flags & MNT_MULTILABEL)     res =3D catopt(res, "multilabel")=
;
>         if (flags & MNT_ACLS)           res =3D catopt(res, "acls");
>         if (flags & MNT_NFS4ACLS)       res =3D catopt(res, "nfsv4acls");
> +       if (flags & MNT_UNTRUSTED)      res =3D catopt(res, "untrusted");
>
>         return (res);
>  }
>
> Modified: head/sys/sys/mount.h
>
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/sys/mount.h        Mon Jul  1 22:11:56 2019        (r349588)
> +++ head/sys/sys/mount.h        Mon Jul  1 23:22:26 2019        (r349589)
> @@ -296,6 +296,7 @@ void          __mnt_vnode_markerfree_active(struct vn=
o
>  #define        MNT_NOCLUSTERW  0x0000000080000000ULL /* disable cluster
> write */
>  #define        MNT_SUJ         0x0000000100000000ULL /* using journaled
> soft updates */
>  #define        MNT_AUTOMOUNTED 0x0000000200000000ULL /* mounted by
> automountd(8) */
> +#define        MNT_UNTRUSTED   0x0000000800000000ULL /* filesys metadata
> untrusted */
>
>  /*
>   * NFS export related mount flags.
> @@ -333,7 +334,8 @@ void          __mnt_vnode_markerfree_active(struct vn=
o
>                         MNT_NOCLUSTERW  | MNT_SUIDDIR   | MNT_SOFTDEP   |=
 \
>                         MNT_IGNORE      | MNT_EXPUBLIC  | MNT_NOSYMFOLLOW
> | \
>                         MNT_GJOURNAL    | MNT_MULTILABEL | MNT_ACLS     |=
 \
> -                       MNT_NFS4ACLS    | MNT_AUTOMOUNTED | MNT_VERIFIED)
> +                       MNT_NFS4ACLS    | MNT_AUTOMOUNTED | MNT_VERIFIED =
|
> \
> +                       MNT_UNTRUSTED)
>
>  /* Mask of flags that can be updated. */
>  #define        MNT_UPDATEMASK (MNT_NOSUID      | MNT_NOEXEC    | \
> @@ -342,7 +344,7 @@ void          __mnt_vnode_markerfree_active(struct vn=
o
>                         MNT_NOSYMFOLLOW | MNT_IGNORE    | \
>                         MNT_NOCLUSTERR  | MNT_NOCLUSTERW | MNT_SUIDDIR  |=
 \
>                         MNT_ACLS        | MNT_USER      | MNT_NFS4ACLS  |=
 \
> -                       MNT_AUTOMOUNTED)
> +                       MNT_AUTOMOUNTED | MNT_UNTRUSTED)
>
>  /*
>   * External filesystem command modifier flags.
>
> Modified: head/sys/ufs/ffs/ffs_vfsops.c
>
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/ufs/ffs/ffs_vfsops.c       Mon Jul  1 22:11:56 2019
> (r349588)
> +++ head/sys/ufs/ffs/ffs_vfsops.c       Mon Jul  1 23:22:26 2019
> (r349589)
> @@ -145,7 +145,7 @@ static struct buf_ops ffs_ops =3D {
>  static const char *ffs_opts[] =3D { "acls", "async", "noatime",
> "noclusterr",
>      "noclusterw", "noexec", "export", "force", "from", "groupquota",
>      "multilabel", "nfsv4acls", "fsckpid", "snapshot", "nosuid", "suiddir=
",
> -    "nosymfollow", "sync", "union", "userquota", NULL };
> +    "nosymfollow", "sync", "union", "userquota", "untrusted", NULL };
>
>  static int
>  ffs_mount(struct mount *mp)
> @@ -184,6 +184,9 @@ ffs_mount(struct mount *mp)
>                 return (error);
>
>         mntorflags =3D 0;
> +       if (vfs_getopt(mp->mnt_optnew, "untrusted", NULL, NULL) =3D=3D 0)
> +               mntorflags |=3D MNT_UNTRUSTED;
> +
>         if (vfs_getopt(mp->mnt_optnew, "acls", NULL, NULL) =3D=3D 0)
>                 mntorflags |=3D MNT_ACLS;
>
>
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpWgbnRyC95yY4orMT69dzS=3AazgPE_ZiROYrDHKcaHgQ>