linux-kernel - Re: [announce] vfs-scale git tree update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87aaj6u3oi.fsf@linux.vnet.ibm.com>
Date:	Wed, 12 Jan 2011 10:19:33 +0530
From:	"Aneesh Kumar K. V" <aneesh.kumar@...ux.vnet.ibm.com>
To:	Ian Kent <raven@...maw.net>, aelder@....com
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Nick Piggin <npiggin@...nel.dk>,
	Al Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [announce] vfs-scale git tree update

On Wed, 12 Jan 2011 11:59:36 +0800, Ian Kent <raven@...maw.net> wrote:
> On Tue, 2011-01-11 at 11:57 -0600, Alex Elder wrote:
> > On Tue, 2011-01-11 at 08:51 -0800, Linus Torvalds wrote:
> > > On Tue, Jan 11, 2011 at 8:34 AM, Alex Elder <aelder@....com> wrote:
> > > >
> > > > FYI, when using this code, as merged by Linus, I hit the
> > > > BUG_ON() at the beginning of d_set_d_op() when it's called
> > > > by autofs4_dir_mkdir().  I managed to work around it by
> > > > just commenting out those BUG_ON() calls but it's something
> > > > that ought to get addressed properly.
> > > 
> > > Yeah, removing the BUG_ON() isn't the right thing to do - it means
> > > that autofs4 is obviously setting the dentry ops twice for the same
> > > dentry.
> > > 
> > > Possibly the thing could be relaxed to allow setting the _same_ d_op
> > > pointer, ie do something like
> > > 
> > >    if (dentry->d_op == op)
> > >       return;
> > > 
> > > at the top of that function. But looking at it, I don't think that
> > > fixes the autofs4 issue.
> > 
> > That's easy enough, but it seems everybody else ensures
> > this gets done just once per dentry, and it would be nice
> > to preserve that "tightness" if possible.
> > 
> > > The fact that autofs4 does "d_add()" before it sets the d_ops (or
> > > other dentry state, for that matter) looks a bit scary. To me that
> > > smells like it might get a  dentry lookup hit before it's actually
> > > fully done.
> > 
> > Agreed.
> 
> Isn't the parent i_mutex held during mkdir()?
> Still the order can be changed, of course.
> 
> > 
> > > Does it make any difference if you move the various d_add() calls down
> > > to the end of the functions to when the "dentry" has really been
> > > instantiated?
> > 
> > Looking at it quickly, I don't think that would matter for
> > the case at hand.  I.e., that might be safer but it doesn't
> > address the fact that these fields are getting initialized
> > multiple times.
> 
> Yeah, a hangover from changes done over time.
> Not setting the dentry op in ->lookup() should fix this.
> 

for 9p I did the other way round. lookup now set the dentry op for both
negative and positive dentries and rest of the place we dropped
d_set_d_op (b8b80cf37c7f0e32729262f805bc0fa81c3e9d12 have the details)

-aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/