On 2019-11-13, Al Viro [off-list ref] wrote:

On Wed, Nov 13, 2019 at 01:44:14PM +1100, Aleksa Sarai wrote:

quoted

On 2019-11-13, Al Viro [off-list ref] wrote:

quoted

On Tue, Nov 05, 2019 at 08:05:49PM +1100, Aleksa Sarai wrote:

quoted

@@ -2277,12 +2277,20 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
 
 	nd->m_seq = read_seqbegin(&mount_lock);
 
-	/* Figure out the starting path and root (if needed). */
-	if (*s == '/') {
+	/* Absolute pathname -- fetch the root. */
+	if (flags & LOOKUP_IN_ROOT) {
+		/* With LOOKUP_IN_ROOT, act as a relative path. */
+		while (*s == '/')
+			s++;

Er...  Why bother skipping slashes?  I mean, not only link_path_walk()
will skip them just fine, you are actually risking breakage in this:
                if (*s && unlikely(!d_can_lookup(dentry))) {
                        fdput(f);
                        return ERR_PTR(-ENOTDIR);
                }
which is downstream from there with you patch, AFAICS.

I switched to stripping the slashes at your suggestion a few revisions
ago[1], and had (wrongly) assumed we needed to handle "/" somehow in
path_init(). But you're quite right about link_path_walk() -- and I'd be
more than happy to drop it.

That, IIRC, was about untangling the weirdness around multiple calls of
dirfd_path_init() and basically went "we might want just strip the slashes
in case of that flag very early in the entire thing, so that later the
normal logics for absolute/relative would DTRT".

Ah okay, I'd misunderstood the point you were making in that thread.

Since your check is right next to checking for absolute pathnames (and
not in the very beginning of path_init()), we might as well turn the
check for absolute pathname into *s == '/' && !(flags &
LOOKUP_IN_ROOT) and be done with that.

Yup, agreed.

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