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 20:53:46 +0000	[thread overview]
Message-ID: <20111206205346.GK2203@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFybiSeYk57DdWXVWJvDk0VTU2hUtJWZUMaBdWOxYtdd_w@mail.gmail.com>

On Tue, Dec 06, 2011 at 11:54:16AM -0800, Linus Torvalds wrote:
> Is AA *worth* that special case? Does it even care that deeply?
> 
> So my argument is that I think your change to make 'root' const an
> dnot be changed is good, and should stay. But 'bastard' should just go
> away, amusingly named or not. All sane callers already call it with a
> NULL pointer. The one case that doesn't isn't worth worrying about,
> and should strive to solve its problems some saner way.
> 
> John explicitly cc'd, since he's the one that would have to figure out
> that /sys special case. Is it possible that just calling __d_path()
> with the global system root would be sufficient for apparmor in this
> case to figure out the /sys part?

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".

That's why __d_path() modifying root had been asking for trouble (besides
being in bad taste); the really are situations when it's very tempting
to look at the place where it had stopped.

Hell knows...  We definitely want to be able to distinguish global roots from
detached vfsmounts.  We _might_ want to distinguish the root of our namespace
from those of others, but I'm less sure about that.

And we don't have a lot of channels for returning extra information, short
of passing a pointer to store it in.  In which case we might bloody well
return the entire vfsmount/dentry pair and let the caller sort it out.

Don't get me wrong, I'm not particulary enthusiastic about that variant;
it's just that I don't see an alternative that would be any better.  I mean,
__d_path(path, root, buf, buflen, &enum_what_kind_of_place_have_I_ended_up_in)?
Can be done, but that'll be just as ugly *and* we'll end up with a growing
zoo of those "kinds" over the next few years... ;-/

  reply	other threads:[~2011-12-06 20:53 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 [this message]
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
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=20111206205346.GK2203@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).