public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	xfs@oss.sgi.com
Subject: Re: inode_permission NULL pointer dereference in 3.13-rc1
Date: Fri, 29 Nov 2013 06:59:41 +0000	[thread overview]
Message-ID: <20131129065941.GW10323@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20131129041416.GV10323@ZenIV.linux.org.uk>

On Fri, Nov 29, 2013 at 04:14:16AM +0000, Al Viro wrote:

> And yes, it has fixed the problem with generic/234.  I'll do full xfstests
> run to see if there's anything else, but this one is obviously needed.
> I'll send it with sane commit message (along with follow_dotdot_rcu()
> fix) later tonight.  path_init() race is a separate story - that one should
> probably go separately, since we'll want it in all branches starting with
> early 2011 or so.

OK, it survives.  However, looking a bit more at follow_dotdot_rcu()...
AFAICS, we have a narrow oopsable race, from 2.6.38 and to 3.12 - think what
happens if we are walking through /tmp/foo/bar/../baz in RCU mode and we'd just
reached /tmp/foo/bar.  handle_dotdot() is called, calling follow_dotdot_rcu().
OK, we are not about to cross a mountpoint.  Read ->d_seq of /tmp/foo into
seq, check that nd->seq matches /tmp/foo/bar (it does, everything's fine)
and set nd->path.dentry to /tmp/foo, with nd->seq set to seq.  Then
we check if the /tmp/foo is overmounted by something; it isn't and now we set
nd->inode.

Sure, it's _very_ hard to get into trouble here - we need somebody to remove
/tmp/foo/bar *and* /tmp/foo while we'd been walking vfsmount hash,
but in theory it is not impossible to get NULL nd->inode.  Then
link_path_walk() gets to checking that we have a directory and we get
an oops on checking inode flags.

I really don't like the way we have nd->inode updates scattered all over
the place in fs/namei.c ;-/  I'm looking into possible ways to deal
with it sanely, but that'll have to wait for tomorrow...

Anyway, I've pushed the minimal regression fix to vfs.git; please, pull
from
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus

Shortlog:
Al Viro (1):
      fix bogus path_put() of nd->root after some unlazy_walk() failures

Diffstat:
 fs/namei.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2013-11-29  6:59 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-24 14:04 inode_permission NULL pointer dereference in 3.13-rc1 Christoph Hellwig
2013-11-24 15:27 ` Al Viro
2013-11-25 16:06   ` Christoph Hellwig
2013-11-26 13:11     ` Al Viro
2013-11-26 14:12       ` Christoph Hellwig
2013-11-27  6:43         ` Al Viro
2013-11-27 10:09           ` Christoph Hellwig
2013-11-28 16:26             ` Al Viro
2013-11-28 21:23               ` Al Viro
2013-11-28 22:51                 ` Dave Chinner
2013-11-28 23:44                   ` Al Viro
2013-11-29  1:46                     ` Dave Chinner
2013-11-29  2:07                       ` Al Viro
2013-11-29  2:17                         ` Linus Torvalds
2013-11-29  2:07                     ` Linus Torvalds
2013-11-29  2:41                       ` Al Viro
2013-11-29  3:59                         ` Al Viro
2013-11-29  4:06                           ` Al Viro
2013-11-29  4:14                             ` Al Viro
2013-11-29  6:59                               ` Al Viro [this message]
2013-11-29 19:44                                 ` Greg KH
2013-11-29 20:17                                   ` Linus Torvalds
2013-11-29 23:55                                     ` Al Viro
2013-11-30  0:18                                       ` Linus Torvalds
2013-11-30 15:09                                 ` [GIT PULL] " Theodore Ts'o
2013-11-30 15:13                                   ` Theodore Ts'o
2013-11-27 21:51           ` Dave Chinner
2013-11-28 15:21 ` Theodore Ts'o
2013-11-28 15:36   ` Theodore Ts'o

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=20131129065941.GW10323@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=xfs@oss.sgi.com \
    /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