All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: John Johansen <john.johansen@canonical.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [git pull] apparmor fix for __d_path() misuse
Date: Wed, 7 Dec 2011 01:10:47 +0000	[thread overview]
Message-ID: <20111207011047.GQ2203@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFyaz7K4V5xLmaQ9GK=5LA1mbRdVGCFspET=y6ode_i=jg@mail.gmail.com>

On Tue, Dec 06, 2011 at 04:42:00PM -0800, Linus Torvalds wrote:

> This part is still just pure and utter sh*t.
> 
> You have not explained why that information is *ever* valid. And I
> claim it isn't.
> 
> We have a bug in our current __d_path(). And I claim that the
> underlying cause of the bug is the crazy "let's return this
> nonsensical and idiotic information that cannot possibly make sense to
> anybody".
> 
> We shouldn't have done that in the first place. And we certainly
> shouldn't *continue* doing that.

Sigh...  This is what it boils down to: there are 3 very different cases -
we'd walked to a global root, we'd raced with umount and we are someplace
never mounted at all.  Case 1 is fine; if apparmor cares whose namespace
it is, it can bloody well check path->mnt itself.  Case 2 is one where
I think that returning pathname does more damage than good; it's really
random in that case and returning NULL is the best thing we can do.
So far, so good, and we don't need to return *any* references to vfsmounts.

Unfortunately, there's also case 3.  Internal vfsmounts.  And that's where
it hits the fan.  Oh, wait...

Guys, I think I know how to deal with that crap.  We *CAN* recognize
internal vfsmounts just fine.  It's right there in ->mnt_flags.  And
in that case bothering with __d_path() and correcting it post-factum is
just plain wrong.

So let's add d_absolute_path(path, buf, buflen).  Having it check that
we'd walked to something mounted.  And returning NULL otherwise.  _Never_
mangle the pathname; replace that procfs weirdness in apparmor with
"Is our path on internal vfsmount?  If so, use dentry_path() on dentry
part and slap /proc/ in front if it was procfs" and that's it.

  reply	other threads:[~2011-12-07  1:10 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
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 [this message]
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=20111207011047.GQ2203@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 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.