lkml.org 
[lkml]   [2020]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCHSET] sanitized pathwalk machinery (v2)
On 2020-02-25, Al Viro <viro@zeniv.linux.org.uk> wrote:
> 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.

Oh boy...

> 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...

You're quite right -- LOOKUP_NO_XDEV should block that and I missed it.

> 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...

You can't go through procfs symlinks with LOOKUP_BENEATH, but if it's
possible to do that kind of jump then it should also be blocked (but I
would say that I'd prefer "block any kind of weird jump").

> 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*...

I think LOOKUP_NO_XDEV should block that since you end up crossing a
mountpoint.

--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2020-02-28 02:26    [W:0.630 / U:1.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site