Re: [PATCH v9 05/10] namei: O_BENEATH-style path resolution flags

From: Aleksa Sarai <cyphar@cyphar.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Alexei Starovoitov <ast@kernel.org>,
	linux-kernel@vger.kernel.org, David Howells <dhowells@redhat.com>,
	linux-kselftest@vger.kernel.org, sparclinux@vger.kernel.org,
	Shuah Khan <shuah@kernel.org>,
	raven@themaw.net, linux-arch@vger.kernel.org,
	linux-s390@vger.kernel.org, Tycho Andersen <tycho@tycho.ws>,
	paul@paul-moore.com, Aleksa Sarai <asarai@suse.de>,
	linux-arm-kernel@lists.infradead.org, linux-mips@vger.kernel.org,
	linux-xtensa@linux-xtensa.org, Kees Cook <keescook@chromium.org>,
	Arnd Bergmann <arnd@arndb.de>, Jann Horn <jannh@google.com>,
	linuxppc-dev@lists.ozlabs.org, linux-m68k@lists.linux-m68k.org,
	Andy Lutomirski <luto@kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	David Drysdale <drysdale@google.com>,
	Christian Brauner <christian@brauner.io>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	linux-parisc@vger.kernel.org, rgb@redhat.com,
	linux-api@vger.kernel.org, Chanho Min <chanho.min@lge.com>,
	Jeff Layton <jlayton@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
	Eric Biederman <ebiederm@xmission.com>,
	linux-alpha@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	containers@lists.linux-foundation.org
Subject: Re: [PATCH v9 05/10] namei: O_BENEATH-style path resolution flags
Date: Tue, 16 Jul 2019 18:03:38 +1000	[thread overview]
Message-ID: <20190716080338.al4cnwdfvdbpzh3r@yavin> (raw)
In-Reply-To: <20190714035826.GQ17978@ZenIV.linux.org.uk>

[-- Attachment #1.1: Type: text/plain, Size: 2482 bytes --]

On 2019-07-14, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sat, Jul 13, 2019 at 03:41:53AM +0100, Al Viro wrote:
> > On Fri, Jul 12, 2019 at 04:00:26PM +0100, Al Viro wrote:
> > > On Fri, Jul 12, 2019 at 02:25:53PM +0100, Al Viro wrote:
> > > 
> > > > 	if (flags & LOOKUP_BENEATH) {
> > > > 		nd->root = nd->path;
> > > > 		if (!(flags & LOOKUP_RCU))
> > > > 			path_get(&nd->root);
> > > > 		else
> > > > 			nd->root_seq = nd->seq;
> > > 
> > > BTW, this assignment is needed for LOOKUP_RCU case.  Without it
> > > you are pretty much guaranteed that lazy pathwalk will fail,
> > > when it comes to complete_walk().
> > > 
> > > Speaking of which, what would happen if LOOKUP_ROOT/LOOKUP_BENEATH
> > > combination would someday get passed?
> > 
> > I don't understand what's going on with ->r_seq in there - your
> > call of path_is_under() is after having (re-)sampled rename_lock,
> > but if that was the only .. in there, who's going to recheck
> > the value?  For that matter, what's to guarantee that the thing
> > won't get moved just as you are returning from handle_dots()?
> > 
> > IOW, what does LOOKUP_IN_ROOT guarantee for caller (openat2())?
> 
> Sigh...  Usual effects of trying to document things:
> 
> 1) LOOKUP_NO_EVAL looks bogus.  It had been introduced by commit 57d4657716ac
> (audit: ignore fcaps on umount) and AFAICS it's crap.  It is set in
> ksys_umount() and nowhere else.  It's ignored by everything except
> filename_mountpoint().  The thing is, call graph for filename_mountpoint()
> is
> 	filename_mountpoint()
> 		<- user_path_mountpoint_at()
> 			<- ksys_umount()
> 		<- kern_path_mountpoint()
> 			<- autofs_dev_ioctl_ismountpoint()
> 			<- find_autofs_mount()
> 				<- autofs_dev_ioctl_open_mountpoint()
> 				<- autofs_dev_ioctl_requester()
> 				<- autofs_dev_ioctl_ismountpoint()
> In other words, that flag is basically "was filename_mountpoint()
> been called by umount(2) or has it come from an autofs ioctl?".
> And looking at the rationale in that commit, autofs ioctls need
> it just as much as umount(2) does.  Why is it not set for those
> as well?  And why is it conditional at all?

In addition, LOOKUP_NO_EVAL == LOOKUP_OPEN (0x100). Is that meant to be
the case? Also I just saw you have a patch in work.namei that fixes this
up -- do you want me to rebase on top of that?

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel