From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg KH Subject: Re: inode_permission NULL pointer dereference in 3.13-rc1 Date: Fri, 29 Nov 2013 11:44:38 -0800 Message-ID: <20131129194438.GA11052@kroah.com> References: <20131128162618.GO10323@ZenIV.linux.org.uk> <20131128212301.GP10323@ZenIV.linux.org.uk> <20131128225102.GS10988@dastard> <20131128234441.GQ10323@ZenIV.linux.org.uk> <20131129024121.GS10323@ZenIV.linux.org.uk> <20131129035939.GT10323@ZenIV.linux.org.uk> <20131129040658.GU10323@ZenIV.linux.org.uk> <20131129041416.GV10323@ZenIV.linux.org.uk> <20131129065941.GW10323@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Linus Torvalds , Dave Chinner , Christoph Hellwig , linux-fsdevel , xfs@oss.sgi.com To: Al Viro Return-path: Received: from mail.linuxfoundation.org ([140.211.169.12]:41245 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751662Ab3K2Tnp (ORCPT ); Fri, 29 Nov 2013 14:43:45 -0500 Content-Disposition: inline In-Reply-To: <20131129065941.GW10323@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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