linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	John Johansen <john.johansen@canonical.com>
Subject: Re: [git pull] apparmor fix for __d_path() misuse
Date: Tue, 6 Dec 2011 21:41:26 +0000	[thread overview]
Message-ID: <20111206214126.GL2203@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFwbWUBs3Fy1hyP3CQ15rF2opvjxLR8gnSF5gERDGCGhOw@mail.gmail.com>

On Tue, Dec 06, 2011 at 01:07:56PM -0800, Linus Torvalds wrote:
> On Tue, Dec 6, 2011 at 12:53 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > The trouble is, might very well stop *NOT* at the global root. ?Consider
> > a race with umount -l; we have no way to tell "it had been outside of
> > chroot jail" from "it had walked up to the place where ->mnt_parent had
> > been already reset, sorry, no idea what it was".
> 
> Sure, but you made that case return NULL already as part of the "no
> bastard" case, didn't you?
> 
> That part of the patch looked fine.
> 
> It was just the extra convolutions around 'bastard' that seemed to be
> over-designed, and made for just a single use that seems very
> peripheral anyway.
> 
> Apart from AppArmor, afaik nobody even really cared where they ended
> up, and even AppArmor really didn't want to know - it just had this
> totally crazy special case about "/sys".

It's not /sys, actually, it's implicit /proc/sys/something from sysctl() ;-/

Hell knows.  Frankly, the whole "let's build a pathname and decide basing
on it" thing had been insane from the very beginning.  For many reasons,
starting with "pathname in which namespace?" and continuing through "who said
that it still means what it used to at the moment of operation" to "what
if it's not a part of any namespace anymore/had never been in one".

Hey, it wasn't me who insisted that pathname-based stuff made any sense
whatsoever...  But "path to the nearest thing that has no ancestor now"
is _meaningless_ if we know nothing about that ancestor.  Cases include
	* that ancestor is the root of our namespace.
	* that ancestor is a solitary vfsmount we'd started in, never had
been mounted in any namespace (e.g. procfs internal vfsmount, etc.)
	* that ancestor is a random vfsmount in a subtree that had been
hit by umount -l, just as we'd been looking at it.  Might be its root,
might be equal to path->mnt, might be something in between.
	* cases 1 and 3 in another process' namespace

In case 3 and analogs the path we'd obtained is pure junk, so I can agree
with "let's just return NULL on those".  The trouble is, how do you tell
(3) from (2)?  And do we want (1)-not-our-namespace to be distinguished
from (1)-our-namespace?

  reply	other threads:[~2011-12-06 21:41 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-06 15:48 [git pull] apparmor fix for __d_path() misuse Al Viro
2011-12-06 16:41 ` Al Viro
2011-12-06 17:21   ` Linus Torvalds
2011-12-06 19:54 ` Linus Torvalds
2011-12-06 20:53   ` Al Viro
2011-12-06 21:07     ` Linus Torvalds
2011-12-06 21:41       ` Al Viro [this message]
2011-12-06 22:48         ` John Johansen
2011-12-06 22:19       ` John Johansen
2011-12-06 22:41         ` Al Viro
2011-12-06 23:12           ` John Johansen
2011-12-06 23:45             ` Linus Torvalds
2011-12-07  0:09               ` John Johansen
2011-12-07  0:16               ` Al Viro
2011-12-07  0:39                 ` Al Viro
2011-12-07  0:42                   ` Linus Torvalds
2011-12-07  1:10                     ` Al Viro
2011-12-07  1:37                       ` Al Viro
2011-12-07  1:44                         ` Al Viro
2011-12-07  2:21                         ` Linus Torvalds
2011-12-07  3:23                           ` Al Viro
2011-12-07  3:11                         ` John Johansen
2011-12-07  4:26                           ` John Johansen
2011-12-07  4:45                             ` Al Viro
2011-12-07  4:59                               ` Al Viro
2011-12-07  3:26                         ` Tetsuo Handa
2011-12-07  3:42                           ` Al Viro
2011-12-07  5:01                             ` Tetsuo Handa
2011-12-07  5:19                               ` Al Viro
2011-12-07  5:44                                 ` Tetsuo Handa
2011-12-07  6:54                                   ` Al Viro
2011-12-07  8:59                                     ` Tetsuo Handa
2011-12-07 16:32                                       ` Al Viro
2011-12-07 17:51                                       ` Al Viro
2011-12-07  0:39                 ` Linus Torvalds
2011-12-07  0:52                   ` Al Viro
2011-12-07  1:11                     ` Linus Torvalds
2011-12-07  1:23                       ` Al Viro
2011-12-07  2:02                         ` Linus Torvalds
2011-12-07  2:17                           ` Al Viro
2011-12-07  2:29                             ` Linus Torvalds

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=20111206214126.GL2203@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=john.johansen@canonical.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).