Re: [PATCH] fs: Validate flags and capabilities before looking up path in ksys_umount

[Al Viro <viro@zeniv.linux.org.uk>]

On Wed, Dec 23, 2020 at 02:26:04AM -0800, Sargun Dhillon wrote:
> ksys_umount was refactored to into split into another function
> (path_umount) to enable sharing code. This changed the order that flags and
> permissions are validated in, and made it so that user_path_at was called
> before validating flags and capabilities.
> 
> Unfortunately, libfuse2[1] and libmount[2] rely on the old flag validation
> behaviour to determine whether or not the kernel supports UMOUNT_NOFOLLOW.
> The other path that this validation is being checked on is
> init_umount->path_umount->can_umount. That's all internal to the kernel.
> 
> [1]: https://github.com/libfuse/libfuse/blob/9bfbeb576c5901b62a171d35510f0d1a922020b7/util/fusermount.c#L403
> [2]: https://github.com/karelzak/util-linux/blob/7ed579523b556b1270f28dbdb7ee07dee310f157/libmount/src/context_umount.c#L813

Sorry, I don't like that solution.  If nothing else, it turns path_umount() into
a landmine for the future.  Yes, we have a regression, yes, we need to do something
about it, but that's not a good way to do that.

FWIW, I would rather separate the check of flags validity from can_umount()
and lift _that_ into ksys_umount(), with "path_umount() relies upon the
flags being minimally sane" comment slapped at path_umount() definition.
The rest of can_umount() really shouldn't be taken out of there.