public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: john.johansen@canonical.com, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, torvalds@linux-foundation.org
Subject: Re: [git pull] apparmor fix for __d_path() misuse
Date: Wed, 7 Dec 2011 05:19:08 +0000	[thread overview]
Message-ID: <20111207051908.GA2203@ZenIV.linux.org.uk> (raw)
In-Reply-To: <201112070501.pB751LoP064331@www262.sakura.ne.jp>

On Wed, Dec 07, 2011 at 02:01:21PM +0900, Tetsuo Handa wrote:
> Al Viro wrote:
> > > How commonly can conditions that make d_absolute_path() return -EINVAL happen?
> > 
> > Race with umount -l, basically.
> 
> d_absolute_path() will return -EINVAL if lazy unmount happens. I see.
> 
> Then, I prefer not denying the request with -EINVAL no matter how unreliable
> the returned pathname is. I don't want to deny the request unless -ENOMEM
> happens or rejected by the policy.
> 
> > In that case the pathname is completely
> > unreliable - if I do umount -l /mnt, pathnames that would be under mnt
> > may get truncated on *ANY* mountpoint.  Not "always cut on /mnt"; not "always
> > cut on the last mountpoint"; it's "everything from root to arbitrary mountpoint
> > on that path is not noticed".
> 
> Unfortunate specification for pathname based access control.
> But since I assume that multiple LSM modules can run in parallel
> ( http://sourceforge.jp/projects/tomoyo/docs/lca2009-kumaneko.pdf),
> I leave more stricter decisions to inode based access control.
> 
> So, can we keep behavior of tomoyo_get_absolute_path() unchanged?

Sure, you are always free to add
	if (pos == ERR_PTR(-EINVAL)) {
		pos = dentry_path(path->dentry, ...)
		/* do whatever you want to buffer to indicate that
		 * beginning had been lost
		 */
	}
since that's the _only_ reliable part of pathname information there is in
such situation.  What should be done to buffer contents is *really* up
to you - what you have there is the path from the root of filesystem path
points to and to path->dentry.  Beginning *is* lost; the thing had been
unmounted and this is all you have.

Or you might want to do __d_path() from (path->mnt,path->mnt->mnt_root) to
path - that's the path from the last mountpoint to your object; i.e. it may
be shorter if that vfsmount had been a binding into the guts of filesystem,
but that is what __d_path() as you used it would stabilize to once the race
window is over.

Again, that's what happens if you are hit with umount and there is *no*
absolute path anymore.  What should be done in such situation is really
up to you - as far as I'm concerned, those races are among the reasons why
pathname-based MAC is a fundamentally wrong idea.

  reply	other threads:[~2011-12-07  5:19 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
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 [this message]
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=20111207051908.GA2203@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=penguin-kernel@I-love.SAKURA.ne.jp \
    --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