Re: [PATCH/RFC] VFS: LOOKUP_MOUNTPOINT should used cached info whenever possible.

[Andreas Dilger <adilger@dilger.ca>]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]


> On Apr 17, 2023, at 9:21 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Mon, 2023-04-17 at 16:24 +0200, Christian Brauner wrote:
>> And I'm curious why is it obvious that we don't want to revalidate _any_
>> path component and not just the last one? Why is that generally safe?
>> Why can't this be used to access files and directories the caller
>> wouldn't otherwise be able to access? I would like to have this spelled
>> out for slow people like me, please.
>> 
>> From my point of view, this would only be somewhat safe _generally_ if
>> you'd allow circumvention for revalidation and permission checking if
>> MNT_FORCE is specified and the caller has capable(CAP_DAC_READ_SEARCH).
>> You'd still mess with overlayfs permission model in this case though.
>> 
>> Plus, there are better options of solving this problem. Again, I'd
>> rather build a separate api for unmounting then playing such potentially
>> subtle security sensitive games with permission checking during path
>> lookup.
> 
> umount(2) is really a special case because the whole intent is to detach
> a mount from the local hierarchy and stop using it. The unfortunate bit
> is that it is a path-based syscall.
> 
> So usually we have to:
> 
> - determine the path: Maybe stat() it and to validate that it's the
>   mountpoint we want to drop

The stat() itself may hang because a remote server, or USB stick is
inaccessible or having media errors.

I've just been having a conversation with Karel Zak to change
umount(1) to use statx() so that it interacts minimally with the fs.

In particular, nfs_getattr() skips revalidate if only minimal attrs
are fetched (STATX_TYPE | STATX_INO), and also skips revalidate if
locally-cached attrs are still valid (STATX_MODE), so this will
avoid yet one more place that unmount can hang.

In theory, vfs_getattr() could get all of these attributes directly
from the vfs_inode in the unmount case.

> - then call umount with that path
> 
> The last thing we want in that case is for the server to decide to
> change some intermediate dentry in between the two operations. Best
> case, you'll get back ENOENT or something when the pathwalk fails. Worst
> case, the server swaps what are two different mountpoints on your client
> and you unmount the wrong one.
> 
> If we don't revaliate, then we're no worse off, and may be better off if
> something hinky happens to the server of an intermediate dentry in the
> path.
> --
> Jeff Layton <jlayton@kernel.org>


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]