Re: [PATCH RFC 0/1] mount: universally disallow mounting over symlinks

[Aleksa Sarai]

On 2020-01-07, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Jan 7, 2020 at 7:13 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> > Another interesting question is whether we want O_PATH open
> > to trigger automounts.
> 
> It does sound like they shouldn't, but as you say:
> 
> >     The thing is, we do *NOT* trigger them
> > (or traverse mountpoints) at the starting point of lookups.
> > I believe it's a mistake (and mine, at that), but I doubt that
> > there's anything that can be done about it at that point.
> > It's a user-visible behaviour [..]
> 
> Hmm. I wonder how set in stone that is. We may have two decades of
> history of not doing it at start point of lookups, but we do *not*
> have two decades of history of O_PATH.
> 
> So what I think we agree would be sane behavior would be for O_PATH
> opens to not trigger automounts (unless there's a slash at the end,
> whatever), but _do_ add the mount-point traversal to the beginning of
> lookups.
> 
> But only do it for the actual O_PATH fd case, not the cwd/root/non-O_PATH case.
> 
> That way we maintain original behavior: if somebody overmounts your
> cwd, you still see the pre-mount directory on lookups, because your
> cwd is "under" the mount.
> 
> But if you open a file with O_PATH, and somebody does a mount
> _afterwards_, the openat() will see that later mount and/or do the
> automount.
> 
> Don't you think that would be the more sane/obvious semantics of how
> O_PATH should work?

If I'm understanding this proposal correctly, this would be a problem
for the libpathrs use-case -- if this is done then there's no way to
avoid a TOCTOU with someone mounting and the userspace program checking
whether something is a mountpoint (unless you have Linux >5.6 and
RESOLVE_NO_XDEV). Today, you can (in theory) do it with MNT_EXPIRE:

  1. Open the candidate directory.
  2. umount2(MNT_EXPIRE) the fd.
    * -EINVAL means it wasn't a mountpoint when we got the fd, and the
	  fd is a stable handle to the underlying directory.
	* -EAGAIN or -EBUSY means that it was a mountpoint or became a
	  mountpoint after the fd was opened (we don't care about that, but
	  fail-safe is better here).
  3. Use the fd from (1) for all operations.

Don't get me wrong, I want to fix this issue *properly* by adding some
new kernel features that allow us to avoid worrying about
mounts-over-magiclinks -- but on old kernels (which libpathrs cares
about) I would be worried about changes like this being backported
resulting in it being not possible to implement the hardening I
mentioned up-thread.

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

Attachment: signature.asc
Description: PGP signature