On Mon, Apr 17, 2023 at 09:25:20PM -0600, Andreas Dilger wrote: > > > On Apr 17, 2023, at 9:21 AM, Jeff Layton <jlayton@xxxxxxxxxx> 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. So we're talking about this commit: https://github.com/util-linux/util-linux/commit/42e141d20505a0deb969c2e583a463c26aadc62f and the patch we discussed here: https://github.com/util-linux/util-linux/issues/2049 > > 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. We don't really need that. As pointed out in that discussion and as Karel did we just want to pass AT_STATX_DONT_SYNC. An api that would allow unmounting by mount id can safely check and retrieve AT_STATX_DONT_SYNC with STATX_ATTR_MOUNT_ROOT and STATX_MNT_ID without ever syncing with the server.
