Support unmounting FUSE mounts

[rianhunter]

FUSE mounts don't need an fstab entry to be unmounted. This checks if a mount is a FUSE mount before checking for the fstab entry, and if so returns success.

Also, make a change to how restricted path canonicalization is done to support FUSE mount points.

[yochananmarqos]

@karelzak This would be awesome to implement.

[karelzak]

It's too late for release v2.33, but I don't see a problem to merge it to the next release.

Well, it will require some changes, for example use read_all() and write_all() in canonicalize_path_restricted() and probably use libmount stuff to get user= and user_id= in is_fuse_mount(), but I'll do these cosmetic changes before the merge.

Thanks.

[rianhunter]

All those changes sound good Karel, thanks for looking into the patch. I can make the changes you're suggesting if that will speed things up for you, let me know.Question about /etc/mtab, is it always a symlink to /proc/mounts on Linux? If so, then no need for the logic to parse the user= option, fuse only sets that when /etc/mtab is a separate file.

[rianhunter]

Hey @karelzak just curious what the status on this was and if there is anything I can do to make this easier for you

[karelzak]

Question about /etc/mtab, is it always a symlink to /proc/mounts on Linux? If so, then no need for the logic to parse the user= option, fuse only sets that when /etc/mtab is a separate file.

The libmount is able to maintain user= option in /run/mount/utab, so it works although you have mtab as a symlink to the /proc/mounts.

The problem I see is the second patch -- it's not secure enough. We have to evaluate mount options again reliable (safe) source (for user= we use /etc/fstab) or we have to be sure that /sbin/umount.type will be really executed, otherwise the library uses umount(2) with uid=0.

It's necessary to call mnt_context_prepare_helper() and make sure cxt->helper is set. I'll try to fix the patch.

[karelzak]

@rianhunter it seems on Fedora is not /sbin/umount.fuse.

How is it expected that umount(8) will umount the FS? The ideal for umount(8) would be ideal to drop permissions and execute /sbin/umount.fuse.

I'd like to implement into libmount something like permanent uhelper=fuse for fuse if user= or user_id= is specified in mount table (mountinfo/mtab or so), but it requires installed /sbin/umount.fuse.

[rianhunter]

@karelzak there is no umount.fuse or umount.fuseblk but you can use fusermount -u. My original goal with this patch was to make fusermount -u obsolete and to just use umount natively. I don't fully understand the security concerns you raise but fusermount -u is usually run as setuid root as far as I know.

EDIT: This patch originally worked by allowing umount(8) to call umount(2) on the mounted directory as setuid root, but only doing so if the suid had the right permissions.

[karelzak]

Well, my idea was to redirect to umount.fuse to avoid any security sensitive decisions for fuse in umount(8) and keep all in fuse code :-)

It's probably paranoid overkill, it seems that user= or user_id= verification is good enough as the options are stored in kernel mount table or in mtab (both is unwritable for non-root users). I'll follow your original patch. Thanks.

[karelzak]

@rianhunter please, try and test topic/fuse-umount branch in util-linux repository. The branch contains your patches after my clean up.

Note you can use

  $ ./configure --enable-static-programs=umount
  $ make umount.static
  # chown root.root umount.static; chmod +s umount.static
  $ ./umount.static <your dir>

to avoid libtools scripts or make install ;-)

[rianhunter]

Hmm this was working for me before on my branch but it doesn't work for me now with your new edits. Strange thing is that it is printing "permission denied" on umount(2) to stderr, but I don't see it ever calling umount(2) in strace:

EDIT: removed strace since it doesn't correctly reflect setuid state

EDIT: Turns out is_fuse_usermount() is incorrectly returning 0. The reason is because optstr = mnt_fs_get_user_options(cxt->fs) is NULL. Not sure why but haven't dug any further.

[karelzak]

Ah, mnt_fs_get_user_options(cxt->fs) works only for user=, but no for user_id=. There should be mnt_fs_get_options() or mnt_fs_get_fs_options(). I'll fix it.

The question is if we really need to support user= in this fuse use-case. The user= is already supported by standard libmount functionality and I guess user= is in the game only because we use it in /etc/fstab. There is no any relation to fuse. (Well, now it does not work, because canonicalize_path_restricted(), should be already fixed by the first your patch...)

Would be enough to support user_id= which is independent on fstab and it is fuse specific and maintained in /proc/self/moutinfo by kernel/libfuse?

I have removed support for user= and updated support for user_id=. Please, try it.

[rianhunter]

@karelzak Yes I think it's enough to only support user_id=. I added the user= logic to mimic what fusermount did but I believe you are correct that umount already handles it elsewhere. If someone actually needs it to be handled in this new way, it can always be added later but for the specific new functionality being added here user_id= is the relevant option.

Edit: Tried your new commit and it works 👍 Nice job

[karelzak]

Merged to the master branch. Thanks for review and tests!