| Message ID | 14ADF290-5B46-44D5-83BC-9AE3732B192C@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Libmount bug ? | expand |
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 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 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. > > 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. > + { "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. Karel
On Oct 14, 2024, at 23:57, Karel Zak <kzak@redhat.com> 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@redhat.com> > http://karelzak.blogspot.com >
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) */ + { "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 */ { "uhelper=", MNT_MS_UHELPER }, /* /sbin/umount.<helper> */