linux-kernel - Re: [git pull] apparmor fix for __d

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111206224100.GM2203@ZenIV.linux.org.uk>
Date:	Tue, 6 Dec 2011 22:41:00 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	John Johansen <john.johansen@...onical.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [git pull] apparmor fix for __d_path() misuse

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@...iv.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/