Re: [PATCH] findmnt: add option to list all statvfs f_flags

[Roberto Bergantinos Corpas <rbergant@redhat.com>]

Hi Karel!

thanks for inputs, appreciate it.

Yes i agree with your objections, in fact first i thought about
extending VFS-OPTIONS via some flag,
and check agains options map, thought i guess i took easy path at some point ;-)

I'll try to see if i can make some work based on your suggestions
before next week.

On Wed, Nov 25, 2020 at 12:21 PM Karel Zak <kzak@redhat.com> wrote:
>
> On Wed, Nov 25, 2020 at 10:45:11AM +0100, Roberto Bergantinos Corpas wrote:
> > It might be useful for security auditing purposes list all possible
> > mount flags/options including default set which are normally not listed.
> >
> > Given that we already call statvfs to retrieve i.e. filesystem size,
> > add an extra column to list all possible f_flags, default or not.
>
> (Sorry Roberto I forgot reply to our private email.)
>
> I understand the goal, but I'm not sure with implementation ;-)
>
> * statvfs() requires access to the mount point, it's better to avoid
>   it if possible due to issue with permissions for non-root users and
>   due to inaccessible NFS, etc.
>
>   The mount options are already accessible for everyone in /proc/self/mountinfo.
>   It's possible to use mnt_optstr_get_flags() to convert the string
>   from mnt_fs_get_vfs_options() to flags.
>
> * your implementation hardcodeds the flags to findmnt.c. This is
>   fragile in terms of future development. And for example right now
>   your list does not include dirsync, sub, silent, strictatime,
>   lazytime, relatime and symfollow options.
>
>   It would be better to use libmount options map where we maintain the
>   option flags and strings, something like
>
>      const struct libmnt_optmap *ent;
>      const struct libmnt_optmap *map;
>
>      map = mnt_get_builtin_optmap(MNT_LINUX_MAP);
>
>      for (ent = map; ent && ent->name; ent++) {
>         if (flags & ent->id)
>             ...
>      }
>
>  Maybe we can add a function to return all (including default) options
>  to libmount to make it easy to use for all applications (something
>  like mnt_fs_get_vfs_alloptions()).
>
> > +     [COL_MNT_OPTIONS]  = { "MNT-OPTIONS",  0.20, SCOLS_FL_TRUNC, N_("All statvfs f_flags from mount") },
>
>  Not sure, but do we really need a new column for this purpose?
>
>  It's still about VFS-OPTIONS, the difference is that you need default
>  build-in options too. What about to add --vfs-show-default (or so)
>  command line options to modify VFS-OPTIONS output?
>
>
>  I can implement it next week (if you don't want play with
>  libmount, etc.).
>
>     Karel
>
> --
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com
>