On Oct 14, 2024, at 23:57, Karel Zak <kzak@xxxxxxxxxx> wrote: > > > Hi Alan, > > On Sat, Oct 12, 2024 at 04:57:39PM GMT, Alan Huang wrote: >> The bcachefs has the helper called mount.bcachefs. > > do you mean the following script? > https://evilpiepirate.org/git/bcachefs-tools.git/tree/mount.bcachefs.sh Not this one, but the Rust code: https://evilpiepirate.org/git/bcachefs-tools.git/tree/src/commands/mount.rs > > I believe that if you call the regular mount(8) from the script, then > it's probably fine to not worry about the options. mount(8) will be > able to ignore them. > >> Currently, there are users using fstab with nofail/user fail to mount, >> we would like to know whether other filesystems using similar helper >> properly handle this. > > The mount.nfs command uses libmount internally to generate the > mount(2) syscall flags, so it is not affected by any additional > options. > > The mount.fuse command has a list of unwanted mount options: > https://github.com/libfuse/libfuse/blob/master/util/mount.fuse.c#L318-L326 It seems that no other kernel filesystems are using similar helpers. > > Please note that the "EXTERNAL HELPERS" section in the mount(8) man > page describes which options are ignored. > > Also, if your mount helper is setuid (like mount.nfs), you still need > to parse fstab to obtain mount options from a safe source. This is > because options from the command line should be ignored as they are > considered unsafe. Good to know. > >> >> This is like commit 06e05eb0f78566b68c44328c37d7c28d8655e9df >> (“libmount: don't pass option "defaults" to helper") >> >> Or would you like something like this? This might be incomplete though (e.g. owner, noowner etc.) >> >> diff --git a/libmount/src/optmap.c b/libmount/src/optmap.c >> index d7569a0f0..c13b9ba19 100644 >> --- a/libmount/src/optmap.c >> +++ b/libmount/src/optmap.c >> @@ -152,11 +152,11 @@ static const struct libmnt_optmap userspace_opts_map[] = >> { "auto", MNT_MS_NOAUTO, MNT_NOHLPS | MNT_INVERT | MNT_NOMTAB }, /* Can be mounted using -a */ >> { "noauto", MNT_MS_NOAUTO, MNT_NOHLPS | MNT_NOMTAB }, /* Can only be mounted explicitly */ >> >> - { "user[=]", MNT_MS_USER }, /* Allow ordinary user to mount (mtab) */ >> - { "nouser", MNT_MS_USER, MNT_INVERT | MNT_NOMTAB }, /* Forbid ordinary user to mount */ >> + { "user[=]", MNT_MS_USER, MNT_NOHLPS}, /* Allow ordinary user to mount (mtab) */ > > This may cause issues with certain helpers (e.g. cifs) where "user=" > is a standard option. However, this is something that needs to be > addressed in libmount, as it already handles this use-case for cifs. > The use of MNT_NOHLPS may override this. Yeah, I was worried that there might be helpers using these options. > >> + { "nouser", MNT_MS_USER, MNT_INVERT | MNT_NOMTAB | MNT_NOHLPS}, /* Forbid ordinary user to mount */ >> >> - { "users", MNT_MS_USERS, MNT_NOMTAB }, /* Allow ordinary users to mount */ >> - { "nousers", MNT_MS_USERS, MNT_INVERT | MNT_NOMTAB }, /* Forbid ordinary users to mount */ >> + { "users", MNT_MS_USERS, MNT_NOMTAB | MNT_NOHLPS}, /* Allow ordinary users to mount */ >> + { "nousers", MNT_MS_USERS, MNT_INVERT | MNT_NOMTAB | MNT_NOHLPS}, /* Forbid ordinary users to mount */ >> >> { "owner", MNT_MS_OWNER, MNT_NOMTAB }, /* Let the owner of the device mount */ >> { "noowner", MNT_MS_OWNER, MNT_INVERT | MNT_NOMTAB }, /* Device owner has no special privs */ >> @@ -180,7 +180,7 @@ static const struct libmnt_optmap userspace_opts_map[] = >> { "sizelimit=", MNT_MS_SIZELIMIT, MNT_NOHLPS | MNT_NOMTAB }, /* loop device size limit */ >> { "encryption=", MNT_MS_ENCRYPTION, MNT_NOHLPS | MNT_NOMTAB }, /* loop device encryption */ >> >> - { "nofail", MNT_MS_NOFAIL, MNT_NOMTAB }, /* Do not fail if ENOENT on dev */ >> + { "nofail", MNT_MS_NOFAIL, MNT_NOMTAB | MNT_NOHLPS}, /* Do not fail if ENOENT on dev */ > > Could this option be usable for some helpers? > > I believe the best solution is to follow the Fuse way and define a > list of options to ignore in your fs-specific helper. > > The ideal solution would be to implement a better libmount (perhaps > libmount2) where the /sbin/mount.<type> helpers are replaced with > dlopen() modules. This way, the library would handle all the details > such as command line and fstab options. Agreed. Thanks, Alan > Karel > > -- > Karel Zak <kzak@xxxxxxxxxx> > http://karelzak.blogspot.com >
