Re: [git pull] apparmor fix for __d_path() misuse

From: Al Viro
Date: Tue Dec 06 2011 - 19:16:46 EST


On Tue, Dec 06, 2011 at 03:45:15PM -0800, Linus Torvalds wrote:
> How about this change:
> - don't change 'root' (and mark it const)
> - if we hit the expected root, we're all happy and do what we do now
> - if we hit some *unexpected* root (the "global root") add a '?' or
> something at the head of the path.
>
> End result: callers like getcwd() can trivially replace their current
> "path_equal(&tmp,&root)" (or whatever they do) with just checking the
> first character of the end result. A good path always starts with '/'.

You get broken /proc/self/mountinfo for chrooted processes with that patch.
You also get /proc/mounts contents change for the same.

Moreover, while we _probably_ can get away with that "prepend '?'",
we'll need to make sure that all checks are comparing with '?', _not_
with '/', or you'll get nasty surprises when __d_path() gets called
on e.g. pipe dentry (pipe:[...]). And while we are at it, we'd better
document that "->d_dname() should never use '?' as the first character"
restriction we've got.

I don't know... playing with magical substrings in what it returns is,
IMO, a bad idea. I really wonder if we'd be better off with just
this:
__d_path(path, root, buf, buflen) - expects non-NULL in
root->mnt, never changes root, returns NULL if path is not under root
d_absolute_path(path, ancestor, buf, buflen) - grabs the
reference to the most remote ancestor it can find, puts pathname
into buf, never returns NULL.

Let tomoyo use that one and path_put(ancestor) afterwards (or look at
it first, if it cares). And let apparmor do the following:
* first call __d_path(), unless asked not to. If it returns
non-NULL, great we've got that path, game over. Otherwise call
d_absolute_path() and log that partial pathname, check where we'd got,
etc. Just remember to path_put(ancestor) after that.

We are trying to shove two different things in one function and result
is ugly; so let's just split it instead of trying to breed weird
hybrids.

Comments?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/