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

On Tue, Dec 06, 2011 at 02:19:08PM -0800, John Johansen wrote:
> On 12/06/2011 01:07 PM, 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.
> > 
> it is, and the plan is to not need the bastard even.  What apparmor should
> be doing is lazy labeling the live objects, and anything that is disconnected
> is evaluated based on the previous labeling.

I still don't understand how you deal with objects seen by processes in
different namespaces.  Please, explain lazy labeling in details...

You do realize that there's no hope whatsoever for prohibiting access
to struct file opened in parent's namespace and already written into
there, right?

> This will also remove its
> use of passing root = { NULL, NULL } to __d_path.

One word: tomoyo...  You are not the only weird one.  They want an absolute
pathname and don't care about races with umount.  Whatever it's relative
to, the thing just goes ahead and plays with itse^Wthe string it got.  As
long as they are not dereferencing pointers to free objects, I Don't Want To
Know(tm).

> Right now we could drop the bastard parameter and passing root = { NULL, NULL }

OK, please tell me what do you need from __d_path().  Suppose it has missed
the supplied root; what do you want to know about the place it had stopped
in?  If we don't do extra arguments, we are limited by this:
	* some condition is checked by __d_path(); if it's satisfied,
pointer to relative pathname (based hell-knows-where) is returned as
usual; we *can't* tell that from normal "reached root, here's your
pathname" case.
	* if it isn't satisfied, we return NULL.  No relative pathname
to look at, etc.

Unless you are OK with what __d_path(path, root, NULL, buf, buflen) is doing
with this patch, we'd need to split it; seq_path_root()/show_mountinfo()
requirements leave no wiggling space for case root->mnt != NULL,
bastard == NULL.

  reply	other threads:[~2011-12-06 22: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
2011-12-06 22:48         ` John Johansen
2011-12-06 22:19       ` John Johansen
2011-12-06 22:41         ` Al Viro [this message]
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=20111206224100.GM2203@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).