linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@redhat.com>
To: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: ebiederm@xmission.com, pavel@ucw.cz, miklos@szeredi.hu,
	viro@ZenIV.linux.org.uk
Subject: [PATCH 0/3] vfs: plug some holes involving LAST_BIND symlinks and file bind mounts (try #5)
Date: Mon, 23 Nov 2009 12:41:21 -0500	[thread overview]
Message-ID: <1258998084-26797-1-git-send-email-jlayton@redhat.com> (raw)

There are a few situations where a lookup can end up returning a dentry
without revalidating it, and without checking whether the calling
process has permissions to access it. Two situations identified so far
are:

1) LAST_BIND symlinks (such as those under /proc/<pid>)

2) file bind mounts

This patchset is intended to fix this by forcing revalidation of the
returned dentries at appropriate locations.

In the case of LAST_BIND symlinks it also adds a check to verify that
the target of the symlink is accessible by the current process by
walking mounts and dentries back up to the root and checking permission
on each inode.

This set fixes the reproducers I have (including the reproducer that
Pavel provided for the permissions bypass). It's still pretty rough
though and I expect that it'll need revision. At this point, I'm mainly
looking to get these questions answered:

1) what should we do if these dentries are found to be invalid? Is it ok
to d_invalidate them? Or is that likely to break something (particularly
in the case of file bind mounts)?

2) I'm using FS_REVAL_DOT as an indicator of whether to force a
d_revalidate. I think that it's appropriate to key off of that flag, but
we may want to rename it (maybe FS_FORCE_D_REVAL ?).

3) is check_path_accessible racy? It seems to work, but something
doesn't seem quite right with this approach. Is this defeatable somehow?
Could a rename of one of the intermediate path components cause
problems?

4) do we need to hold the dcache_lock across the check_path_accessible
call?

This isn't my usual area of expertise, so I'm definitely open to
suggestions on this.

Jeff Layton (3):
  vfs: force reval of target when following LAST_BIND symlinks
  vfs: force reval on dentry of bind mounted files on FS_REVAL_DOT
    filesystems
  vfs: check path permissions on target of LAST_BIND symlinks

 fs/namei.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 102 insertions(+), 2 deletions(-)

             reply	other threads:[~2009-11-23 17:41 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-23 17:41 Jeff Layton [this message]
2009-11-23 17:41 ` [PATCH 1/3] vfs: force reval of target when following LAST_BIND symlinks Jeff Layton
2009-11-23 17:41 ` [PATCH 2/3] vfs: force reval on dentry of bind mounted files on FS_REVAL_DOT filesystems Jeff Layton
2009-11-23 17:41 ` [PATCH 3/3] vfs: check path permissions on target of LAST_BIND symlinks Jeff Layton
2009-11-23 22:05 ` [PATCH 0/3] vfs: plug some holes involving LAST_BIND symlinks and file bind mounts (try #5) Eric W. Biederman
2009-11-23 22:36   ` Jeff Layton
2009-11-23 22:49     ` Jamie Lokier
2009-11-23 23:15       ` Jeff Layton
2009-11-23 23:35         ` Eric W. Biederman
2009-11-24  0:34           ` Jeff Layton
2009-11-24  1:20             ` Jamie Lokier
2009-11-24 11:26               ` Jeff Layton
2009-11-24 11:53                 ` Miklos Szeredi
2009-11-24 12:09                   ` Pavel Machek
2009-11-24 12:59                     ` Miklos Szeredi
2009-11-30 12:28                       ` Pavel Machek
2009-11-30 19:21                         ` Eric W. Biederman
2009-11-24 13:13                     ` Duane Griffin
2009-11-30 19:00                       ` Jamie Lokier
2009-12-01  8:56                         ` Duane Griffin
2009-12-16 12:31         ` Al Viro
2009-12-20 19:59           ` Pavel Machek
2009-12-20 21:04             ` Al Viro
2009-12-20 21:06               ` Pavel Machek
2009-12-20 21:23                 ` Al Viro
2010-01-01 15:40                   ` Pavel Machek
2010-01-10  4:42                     ` Al Viro
2009-12-01 13:15   ` Jeff Layton

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=1258998084-26797-1-git-send-email-jlayton@redhat.com \
    --to=jlayton@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=pavel@ucw.cz \
    --cc=viro@ZenIV.linux.org.uk \
    /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).