All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Aleksa Sarai <cyphar@cyphar.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	David Howells <dhowells@redhat.com>,
	Eric Biederman <ebiederm@xmission.com>,
	stable <stable@vger.kernel.org>,
	Christian Brauner <christian.brauner@ubuntu.com>,
	Serge Hallyn <serge@hallyn.com>,
	dev@opencontainers.org,
	Linux Containers <containers@lists.linux-foundation.org>,
	Linux API <linux-api@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ian Kent <raven@themaw.net>
Subject: Re: [PATCH RFC 0/1] mount: universally disallow mounting over symlinks
Date: Tue, 14 Jan 2020 05:12:48 +0000	[thread overview]
Message-ID: <20200114051248.GX8904@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20200114045733.GW8904@ZenIV.linux.org.uk>

On Tue, Jan 14, 2020 at 04:57:33AM +0000, Al Viro wrote:
> On Sat, Jan 11, 2020 at 08:07:19AM +1100, Aleksa Sarai wrote:
> 
> > 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.
> 
> ... except that foo/../bar *WILL* cross into the covering mount, on any
> kernel that supports ...at(2) at all, so I would be very cautious about
> any kind "hardening" claims in that case.
> 
> I'm not sure about Linus' proposal - it looks rather convoluted and we
> get a hard to describe twist of semantics in an area (procfs symlinks
> vs. mount traversal) on top of everything else in there...

PS: one thing that might be interesting is exposing LOOKUP_DOWN via
AT_... flag - it would allow to request mount traversals at the starting
point explicitly.  Pretty much all code needed for that is already there;
all it would take is checking the flag in path_openat() and path_parentat()
and having handle_lookup_down() called there, same as in path_lookupat().

A tricky question is whether such flag should affect absolute symlinks -
i.e.

chdir /foo
ln -s /bar barf
overmount /
do lookup with that flag for /bar/splat
do lookup with that flag for barf/splat

Do we want the same results in both calls?  The first one would
traverse mounts on / and walk into /bar/splat in overmounting;
the second - see no mounts whatsoever on current directory (/foo
in old root), see the symlink to "/bar", jump to process' root
and proceed from there, first for "bar", then "splat" in it...

  reply	other threads:[~2020-01-14  5:12 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-30  5:20 [PATCH RFC 0/1] mount: universally disallow mounting over symlinks Aleksa Sarai
2019-12-30  5:20 ` [PATCH RFC 1/1] " Aleksa Sarai
2019-12-30  5:20   ` Aleksa Sarai
2019-12-30  7:34   ` Linus Torvalds
2019-12-30  8:28     ` Aleksa Sarai
2020-01-08  4:39       ` Andy Lutomirski
2019-12-30  5:44 ` [PATCH RFC 0/1] " Al Viro
2019-12-30  5:49   ` Aleksa Sarai
2019-12-30  7:29     ` Aleksa Sarai
2019-12-30  7:53       ` Linus Torvalds
2019-12-30  8:32         ` Aleksa Sarai
2020-01-02  8:58           ` David Laight
2020-01-02  9:09             ` Aleksa Sarai
2020-01-01  0:43       ` Al Viro
2020-01-01  0:54         ` Al Viro
2020-01-01  3:08           ` Al Viro
2020-01-01 14:44             ` Aleksa Sarai
2020-01-01 23:40               ` Al Viro
2020-01-02  3:59                 ` Aleksa Sarai
2020-01-03  1:49                   ` Al Viro
2020-01-04  4:46                     ` Ian Kent
2020-01-08  3:13                     ` Al Viro
2020-01-08  3:54                       ` Linus Torvalds
2020-01-08 21:34                         ` Al Viro
2020-01-10  0:08                           ` Linus Torvalds
2020-01-10  4:15                             ` Al Viro
2020-01-10  5:03                               ` Linus Torvalds
2020-01-10  6:20                               ` Ian Kent
     [not found]                                 ` <979cf680b0fbdce515293a3449d564690cde6a3f.camel-PKsaG3nR2I+sTnJN9+BGXg@public.gmane.org>
2020-01-12 21:33                                   ` Al Viro
2020-01-12 21:33                                     ` Al Viro
2020-01-13  2:59                                     ` Ian Kent
2020-01-14  0:25                                       ` Ian Kent
2020-01-14  4:39                                         ` Al Viro
2020-01-14  5:01                                           ` Ian Kent
     [not found]                                             ` <d6cad1552171da1eb38c55d1d7b1ff45902b101f.camel-PKsaG3nR2I+sTnJN9+BGXg@public.gmane.org>
2020-01-14  5:59                                               ` Ian Kent
2020-01-14  5:59                                                 ` Ian Kent
2020-01-10 21:07                         ` Aleksa Sarai
2020-01-14  4:57                           ` Al Viro
2020-01-14  5:12                             ` Al Viro [this message]
     [not found]                             ` <20200114045733.GW8904-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2020-01-14 20:01                               ` Aleksa Sarai
2020-01-14 20:01                                 ` Aleksa Sarai
2020-01-15 14:25                                 ` Al Viro
2020-01-15 14:29                                   ` Aleksa Sarai
2020-01-15 14:34                                     ` Aleksa Sarai
2020-01-15 14:48                                       ` Al Viro
2020-01-15 14:48                                         ` Al Viro
     [not found]                                         ` <20200115144831.GJ8904-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2020-01-18 12:07                                           ` [PATCH v3 0/2] openat2: minor uapi cleanups Aleksa Sarai
2020-01-18 12:07                                             ` Aleksa Sarai
2020-01-18 12:07                                             ` [PATCH v3 1/2] open: introduce openat2(2) syscall Aleksa Sarai
2020-01-19  7:20                                               ` kbuild test robot
     [not found]                                             ` <20200118120800.16358-1-cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
2020-01-18 12:08                                               ` [PATCH v3 2/2] selftests: add openat2(2) selftests Aleksa Sarai
2020-01-18 12:08                                                 ` Aleksa Sarai
2020-01-18 15:28                                             ` [PATCH v3 0/2] openat2: minor uapi cleanups Al Viro
     [not found]                                               ` <20200118152833.GS8904-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2020-01-18 18:09                                                 ` Al Viro
2020-01-18 18:09                                                   ` Al Viro
     [not found]                                                   ` <20200118180941.GT8904-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2020-01-18 23:03                                                     ` Aleksa Sarai
2020-01-18 23:03                                                       ` Aleksa Sarai
2020-01-19  1:12                                                       ` Al Viro
2020-01-15 13:57                             ` [PATCH RFC 0/1] mount: universally disallow mounting over symlinks Aleksa Sarai
2020-01-19  3:14                               ` [RFC][PATCHSET][CFT] pathwalk cleanups and fixes Al Viro
2020-01-19  3:14                                 ` Al Viro
2020-01-19  3:17                                 ` [PATCH 01/17] do_add_mount(): lift lock_mount/unlock_mount into callers Al Viro
2020-01-19  3:17                                   ` [PATCH 02/17] fix automount/automount race properly Al Viro
2020-01-30 14:34                                     ` Christian Brauner
2020-01-19  3:17                                   ` [PATCH 03/17] follow_automount(): get rid of dead^Wstillborn code Al Viro
2020-01-30 14:38                                     ` Christian Brauner
2020-01-19  3:17                                   ` [PATCH 04/17] follow_automount() doesn't need the entire nameidata Al Viro
2020-01-30 14:45                                     ` Christian Brauner
2020-01-30 15:38                                       ` Al Viro
2020-01-30 15:55                                         ` Al Viro
2020-01-19  3:17                                   ` [PATCH 05/17] make build_open_flags() treat O_CREAT | O_EXCL as implying O_NOFOLLOW Al Viro
2020-01-19  3:17                                   ` [PATCH 06/17] handle_mounts(): start building a sane wrapper for follow_managed() Al Viro
2020-01-19  3:17                                   ` [PATCH 07/17] atomic_open(): saner calling conventions (return dentry on success) Al Viro
2020-01-19  3:17                                   ` [PATCH 08/17] lookup_open(): " Al Viro
2020-01-19  3:17                                   ` [PATCH 09/17] do_last(): collapse the call of path_to_nameidata() Al Viro
2020-01-19  3:17                                   ` [PATCH 10/17] handle_mounts(): pass dentry in, turn path into a pure out argument Al Viro
2020-01-19  3:17                                   ` [PATCH 11/17] lookup_fast(): consolidate the RCU success case Al Viro
2020-01-19  3:17                                   ` [PATCH 12/17] teach handle_mounts() to handle RCU mode Al Viro
2020-01-19  3:17                                   ` [PATCH 13/17] lookup_fast(): take mount traversal into callers Al Viro
2020-01-19  3:17                                   ` [PATCH 14/17] new step_into() flag: WALK_NOFOLLOW Al Viro
2020-01-19  3:17                                   ` [PATCH 15/17] fold handle_mounts() into step_into() Al Viro
2020-01-19  3:17                                   ` [PATCH 16/17] LOOKUP_MOUNTPOINT: fold path_mountpointat() into path_lookupat() Al Viro
2020-01-19  3:17                                   ` [PATCH 17/17] expand the only remaining call of path_lookup_conditional() Al Viro
2020-01-19  3:17                                   ` [PATCH 1/9] merging pick_link() with get_link(), part 1 Al Viro
2020-01-19  3:17                                   ` [PATCH 2/9] merging pick_link() with get_link(), part 2 Al Viro
2020-01-19  3:17                                   ` [PATCH 3/9] merging pick_link() with get_link(), part 3 Al Viro
2020-01-19  3:17                                   ` [PATCH 4/9] merging pick_link() with get_link(), part 4 Al Viro
2020-01-19  3:17                                   ` [PATCH 5/9] merging pick_link() with get_link(), part 5 Al Viro
2020-01-19  3:17                                   ` [PATCH 6/9] merging pick_link() with get_link(), part 6 Al Viro
2020-01-19  3:17                                   ` [PATCH 7/9] finally fold get_link() into pick_link() Al Viro
2020-01-19  3:17                                   ` [PATCH 8/9] massage __follow_mount_rcu() a bit Al Viro
2020-01-19  3:17                                   ` [PATCH 9/9] new helper: traverse_mounts() Al Viro
2020-01-30 14:13                                   ` [PATCH 01/17] do_add_mount(): lift lock_mount/unlock_mount into callers Christian Brauner
2020-01-19 14:33                                 ` [RFC][PATCHSET][CFT] pathwalk cleanups and fixes Ian Kent
2020-01-10 23:19                     ` [PATCH RFC 0/1] mount: universally disallow mounting over symlinks Al Viro
2020-01-13  1:48                       ` Ian Kent
2020-01-13  3:54                         ` Al Viro
2020-01-13  6:00                           ` Ian Kent
2020-01-13  6:03                             ` Ian Kent
2020-01-13 13:30                               ` Al Viro
     [not found]                                 ` <20200113133047.GR8904-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2020-01-14  7:25                                   ` Ian Kent
2020-01-14  7:25                                     ` Ian Kent
2020-01-14 12:17                                     ` Ian Kent
2020-01-04  5:52               ` Andy Lutomirski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200114051248.GX8904@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=christian.brauner@ubuntu.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=cyphar@cyphar.com \
    --cc=dev@opencontainers.org \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=raven@themaw.net \
    --cc=serge@hallyn.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.