Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 25 Feb 2020 01:24:57 +0000
From:   Al Viro <viro@zeniv.linux.org.uk>
To:     linux-fsdevel@vger.kernel.org
Cc:     linux-kernel@vger.kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Aleksa Sarai <cyphar@cyphar.com>
Subject: Re: [RFC][PATCHSET] sanitized pathwalk machinery (v2)
Message-ID: <20200225012457.GA138294@ZenIV.linux.org.uk>
References: <20200223011154.GY23230@ZenIV.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200223011154.GY23230@ZenIV.linux.org.uk>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, Feb 23, 2020 at 01:12:21AM +0000, Al Viro wrote:
> 	This is a slightly extended repost of the patchset posted on
> Jan 19.  Current branch is in vfs.git#work.do_last, the main
> difference from the last time around being a bit of do_last()
> untangling added in the end of series.  #work.openat2 is already
> in mainline, which simplifies the series - now it's a straight
> branch with no merges.

Whee...  While trying to massage ".." handling towards the use of
regular mount crossing semantics, I've found something interesting.
Namely, if you start in a directory with overmounted parent,
LOOKUP_NO_XDEV resolution of ../something will bloody well cross
into the overmount.

Reason: follow_dotdot() (and its RCU counterpart) check for LOOKUP_NO_XDEV
when crossing into underlying fs, but not when crossing into overmount
of the parent.

Interpretation of .. is basically

loop:	if we are in root					// uncommon
		next = current position
	else if we are in root of a mounted filesystem		// more rare
		move to underlying mountpoint
		goto loop
	else
		next = parent directory of current position	// most common

	while next is overmounted				// _VERY_ uncommon
		next = whatever's mounted on next

	move to next

The second loop should've been sharing code with the normal mountpoint
crossing.  It doesn't, which has already lead to interesting inconsistencies
(e.g. autofs generally expects ->d_manage() to be called before crossing
into it; here it's not done).  LOOKUP_NO_XDEV has just added one more...

Incidentally, another inconsistency is LOOKUP_BENEATH treatment in case
when we have walked out of the subtree by way of e.g. procfs symlink and
then ran into .. in the absolute root (that's
                if (!follow_up(&nd->path))
                        break;
in follow_dotdot()).  Shouldn't that give the same reaction as ..
in root (EXDEV on LOOKUP_BENEATH, that is)?  It doesn't...

Another one is about LOOKUP_NO_XDEV again: suppose you have process'
root directly overmounted and cwd in the root of whatever's overmounting
it.  Resolution of .. will stay in cwd - we have no parent within the
chroot jail we are in, so we move to whatever's overmounting that root.
Which is the original location.  Should we fail on LOOKUP_NO_XDEV here?
Plain .. in the root of chroot jail (not overmounted by anything) does
*not*...