Re: inode_permission NULL pointer dereference in 3.13-rc1

From: Greg KH <gregkh@linuxfoundation.org>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	xfs@oss.sgi.com
Subject: Re: inode_permission NULL pointer dereference in 3.13-rc1
Date: Fri, 29 Nov 2013 11:44:38 -0800	[thread overview]
Message-ID: <20131129194438.GA11052@kroah.com> (raw)
In-Reply-To: <20131129065941.GW10323@ZenIV.linux.org.uk>

On Fri, Nov 29, 2013 at 06:59:41AM +0000, Al Viro wrote:
> 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

So, should d870b4a191a389c661cd40aacb06981c26b5e504 be queued up for
-stable releases to resolve this issue there as well, or am I
misunderstanding your post above?

thanks,

greg k-h

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