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

On Sat, Nov 16, 2019 at 11:27:56AM +1100, Aleksa Sarai wrote:

quoted

@@ -1383,6 +1398,8 @@ static int follow_dotdot_rcu(struct nameidata *nd)
 				return -ECHILD;
 			if (&mparent->mnt == nd->path.mnt)
 				break;
+			if (unlikely(nd->flags & LOOKUP_NO_XDEV))
+				return -EXDEV;
 			/* we know that mountpoint was pinned */
 			nd->path.dentry = mountpoint;
 			nd->path.mnt = &mparent->mnt;

@@ -1397,6 +1414,8 @@ static int follow_dotdot_rcu(struct nameidata *nd)
 			return -ECHILD;
 		if (!mounted)
 			break;
+		if (unlikely(nd->flags & LOOKUP_NO_XDEV))
+			return -EXDEV;
 		nd->path.mnt = &mounted->mnt;
 		nd->path.dentry = mounted->mnt.mnt_root;
 		inode = nd->path.dentry->d_inode;

I really don't think we should return hard errors from that function.
Let the caller redo it in refwalk mode.

I suspected as much, though my reason for not changing it was that the
mount_lock check should ensure that the cached status of whether ".." is
a mountpoint crossing is correct. But I guess this is more about being
safe than sorry, rather than an actual bug?

It's not the fast path, especially for this kind of errors.  Matter of
fact, I'm not sure about -ENOENT returned in another failure case
there - it's probably OK, but again, -ECHILD would be just as good.

I can switch the -ENOENT too if you like.

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