Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Andreas Dilger <adilger@dilger.ca>
Message-Id: <1AC965F2-BAC6-4D0F-A2A6-C414CDF110AF@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_581956D5-D9DE-48C3-929B-CAF6060706C3";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [PATCH/RFC] VFS: LOOKUP_MOUNTPOINT should used cached info
 whenever possible.
Date:   Mon, 17 Apr 2023 21:25:20 -0600
In-Reply-To: <85774a5de74b2b7828c8b8f7e041f0e9e2bc6094.camel@kernel.org>
Cc:     Christian Brauner <brauner@kernel.org>, NeilBrown <neilb@suse.de>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Dave Wysochanski <dwysocha@redhat.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        linux-nfs <linux-nfs@vger.kernel.org>,
        David Howells <dhowells@redhat.com>,
        Christoph Hellwig <hch@lst.de>, Karel Zak <kzak@redhat.com>
To:     Jeff Layton <jlayton@kernel.org>
References: <95ee689c76bf034fa2fe9fade0bccdb311f3a04f.camel@kernel.org>
 <168168683217.24821.6260957092725278201@noble.neil.brown.name>
 <20230417-beisein-investieren-360fa20fb68a@brauner>
 <6c08ad94ca949d0f3525f7e1fc24a72c50affd59.camel@kernel.org>
 <20230417-relaxen-selektiert-4b4b4143d7f6@brauner>
 <85774a5de74b2b7828c8b8f7e041f0e9e2bc6094.camel@kernel.org>
Precedence: bulk


--Apple-Mail=_581956D5-D9DE-48C3-929B-CAF6060706C3
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=us-ascii


> 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


--Apple-Mail=_581956D5-D9DE-48C3-929B-CAF6060706C3
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAmQ+DaAACgkQcqXauRfM
H+D9bhAArwx6YZ1JWmKrSRDxkjrjKPXa7tALqsqaRKxRH9Ar2aZNhOsQNH/APN9G
+kLPKYXTveRbj2LEvRwkk+O2rpmJ+7u2UFb1/l+gCWnEmlrdS9yQZI4k6cnX/iNW
gD6eXO7172P5ZHkOtQTxlRVt0gAk21h97xAkCF4D/+xgqRnTXUzkpsLmy3082cJH
WHWoXenHnoVMKA1KXZedMS19wvPuoNsJJvVvXNN4K7q28G+PTbKHIamdXuoshW5N
n7EnhCm7CxHpZyLybnrDUWCBDCxWfEv+geBomG/hofsk4IdctfkU0Se92vDtyu9U
LchEF8rP1ckwGEX4Kl5mjePRXRn2eNNIi1Y8XpslmKzljQvdvPerM0NZbIxwz+JJ
HkjBTC/GHBQ1dFUcxJdLb82fF5Lg7H0uqeEOgLgOhxei6C+YcgqnetXMp+VEMInn
ZrqymJhpZWs5WQm3B4aLeA4u5918TBNPknZm1Vk3zl+y2khctkVlF4CE90kkGHC6
fZrPKRk694rFE2l8y5f+tXDPBKsoHYwD6tP9E1bwKNqGatCwXRcX5b3hfqK6Vt0w
znGX0sxxKKKM2drEeqJrIOzb5ztwAFca/FH7tnA1zJnxuq4GCeY6dkmKjqDjgVGe
kT6dcKfsHNHj58FCFmT/yI/LFQJ+rVhqQTMV0uIhh0QmKqHVgEo=
=bL3W
-----END PGP SIGNATURE-----

--Apple-Mail=_581956D5-D9DE-48C3-929B-CAF6060706C3--