linux-kernel - Re: [PATCH RFC v3] vfs: make fstatat retry once on ESTALE errors from getattr call

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 24 Apr 2012 10:50:49 -0400
From:	Jeff Layton <jlayton@...hat.com>
To:	Peter Staubach <pstaubach@...grid.com>
Cc:	Steve Dickson <SteveD@...hat.com>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"miklos@...redi.hu" <miklos@...redi.hu>,
	"viro@...IV.linux.org.uk" <viro@...IV.linux.org.uk>,
	"hch@...radead.org" <hch@...radead.org>,
	"michael.brantley@...haw.com" <michael.brantley@...haw.com>,
	"sven.breuner@...m.fraunhofer.de" <sven.breuner@...m.fraunhofer.de>,
	"chuck.lever@...cle.com" <chuck.lever@...cle.com>,
	"malahal@...ibm.com" <malahal@...ibm.com>,
	"bfields@...ldses.org" <bfields@...ldses.org>,
	"trond.myklebust@....uio.no" <trond.myklebust@....uio.no>,
	"rees@...ch.edu" <rees@...ch.edu>
Subject: Re: [PATCH RFC v3] vfs: make fstatat retry once on ESTALE errors
 from getattr call

On Mon, 23 Apr 2012 16:38:00 -0400
Peter Staubach <pstaubach@...grid.com> wrote:

> I don't really like the idea of introducing another errno as well.  It seems like too much complexity and represents complexity that no one has really justified needing.
> 

I tend to agree here. Miklos, can you elaborate a bit on what fuse
filesystems you're particularly concerned about here? Which ones return
ESTALE and under what conditions. Maybe we can try to tailor this
solution to avoid the complexity without impacting them.

> This is a situation which we know happens in nature.  We should fix it and fix it correctly and not for just "part of the time".  The changes are pretty simple and straightforward, so complexity isn't even an argument.
> 

That was the case in the original patch, yes. One thing I see that will
be tricky in forward porting all of that work is that that set also had
some checks to make sure that lookups were making forward progress
before retrying. Trying to add that to the current lookup code may be
more difficult.

do_path_lookup (for instance) does this currently:

---------------[snip]---------------
                retval = path_lookupat(dfd, name, flags, nd);
        if (unlikely(retval == -ESTALE))
                retval = path_lookupat(dfd, name, flags | LOOKUP_REVAL, nd);
---------------[snip]---------------

The trivial thing to do to make that retry is to turn that ESTALE check
into a while loop (optionally with some sort of limit on the number of
retries). But...we don't have a way to know the pointer to the last
dentry that we successfully looked up.

One possibility there is to try and have the error paths that matter
set the nd->path.dentry to the current one (without taking any
references). Then we could compare that to the last one on each pass of
the while loop and ensure that it's different and just not try to
dereference it.

> A tunable sounds good, until it is needed, and when it is needed, it is too late.  The system should just work correctly on its own, so I don't think that this is such a good idea either.
> 

Yeah, it's not ideal, but if there are concerns about the number of
retries, then that's one way to alleviate them. Eventually we could
eliminate the tunable when/if we found a default that worked for
everyone, or just decide that retrying indefinitely is OK.

-- 
Jeff Layton <jlayton@...hat.com>
--
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/