Re: NFS/d_splice_alias breakage

From	Al Viro <viro@ZenIV.linux.org.uk>
Newsgroups	linux.kernel
Subject	Re: NFS/d_splice_alias breakage
Date	2016-06-03 07:00 +0200
Message-ID	<rFPnc-76D-23@gated-at.bofh.it> (permalink)
References	<rFJUt-3Rl-1@gated-at.bofh.it> <rFO7L-6nc-11@gated-at.bofh.it> <rFOhr-6qj-9@gated-at.bofh.it> <rFOU9-6WX-3@gated-at.bofh.it> <rFPdv-73m-7@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Fri, Jun 03, 2016 at 05:42:53AM +0100, Al Viro wrote:
> On Fri, Jun 03, 2016 at 05:26:48AM +0100, Al Viro wrote:
> 
> > Looks like the right thing to do would be to do d_drop() at no_open:,
> > just before we call nfs_lookup().  If not earlier, actually...  How
> > about the following?
> 
> A bit of rationale: dentry in question is negative and attempt to open
> it has just failed; in case it's really negative we did that d_drop()
> anyway (and then unconditionally rehashed it).  In case when we proceed to
> nfs_lookup() and it does not fail, we'll have it rehashed there (with the
> right inode).  What do we lose from doing d_drop() on *any* error here?
> It's negative, with dubious validity.  In the worst case, we had open
> and lookup fail, but it's just us - the sucker really is negative and
> somebody else would be able to revalidate it.  If we drop it here (and
> not rehash later), that somebody else will have to allocate an in-core
> dentry before doing lookup or atomic_open.  Which is negligible extra
> burden.

I suspect that it got broken in commit 275bb3078 (NFSv4: Move dentry
instantiation into the NFSv4-specific atomic open code).  Prior to that
commit, d_drop() was there (error or no error).  Looks like 3.10+, indeed.
I agree that we shouldn't drop it on successful open, but it needs to be
done on errors.  All of them, not just ENOENT, as in that commit.

Thread

NFS/d_splice_alias breakage Oleg Drokin <green@linuxhacker.ru> - 2016-06-03 01:10 +0200
  [PATCH] Allow d_splice_alias to accept hashed dentries green@linuxhacker.ru - 2016-06-03 02:10 +0200
    Re: [PATCH] Allow d_splice_alias to accept hashed dentries Oleg Drokin <green@linuxhacker.ru> - 2016-06-03 02:30 +0200
  Re: NFS/d_splice_alias breakage Trond Myklebust <trondmy@primarydata.com> - 2016-06-03 02:50 +0200
    Re: NFS/d_splice_alias breakage Oleg Drokin <green@linuxhacker.ru> - 2016-06-03 03:00 +0200
      Re: NFS/d_splice_alias breakage Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 05:30 +0200
        Re: NFS/d_splice_alias breakage Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 05:40 +0200
    Re: NFS/d_splice_alias breakage Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 05:30 +0200
  Re: NFS/d_splice_alias breakage Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 05:40 +0200
    Re: NFS/d_splice_alias breakage Oleg Drokin <green@linuxhacker.ru> - 2016-06-03 05:50 +0200
      Re: NFS/d_splice_alias breakage Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 06:30 +0200
        Re: NFS/d_splice_alias breakage Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 06:50 +0200
          Re: NFS/d_splice_alias breakage Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 07:00 +0200
        Re: NFS/d_splice_alias breakage Oleg Drokin <green@linuxhacker.ru> - 2016-06-03 07:00 +0200
          Re: NFS/d_splice_alias breakage Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 08:00 +0200
            Re: NFS/d_splice_alias breakage Oleg Drokin <green@linuxhacker.ru> - 2016-06-07 01:40 +0200
              Re: NFS/d_splice_alias breakage Oleg Drokin <green@linuxhacker.ru> - 2016-06-10 03:40 +0200
                Re: NFS/d_splice_alias breakage Oleg Drokin <green@linuxhacker.ru> - 2016-06-10 18:50 +0200
        Dcache oops Oleg Drokin <green@linuxhacker.ru> - 2016-06-03 18:40 +0200
          Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 20:30 +0200
            Re: Dcache oops Oleg Drokin <green@linuxhacker.ru> - 2016-06-03 20:40 +0200
              Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 22:10 +0200
                Re: Dcache oops Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-03 23:20 +0200
                Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 23:30 +0200
                Re: Dcache oops Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-04 00:10 +0200
                Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-04 00:30 +0200
                Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-04 00:30 +0200
                Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-04 00:40 +0200
                Re: Dcache oops Oleg Drokin <green@linuxhacker.ru> - 2016-06-04 00:50 +0200
                Re: Dcache oops Oleg Drokin <green@linuxhacker.ru> - 2016-06-04 02:00 +0200
                Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-04 03:00 +0200
                Re: Dcache oops Jeff Layton <jlayton@poochiereds.net> - 2016-06-04 14:30 +0200
                Re: Dcache oops Oleg Drokin <green@linuxhacker.ru> - 2016-06-04 18:20 +0200
                [PATCH] nfs4: Fix potential use after free of state in nfs4_do_reclaim. green@linuxhacker.ru - 2016-06-04 18:30 +0200
                Re: [PATCH] nfs4: Fix potential use after free of state in  nfs4_do_reclaim. Jeff Layton <jlayton@poochiereds.net> - 2016-06-04 22:00 +0200
                Re: Dcache oops Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-04 00:40 +0200
                Re: Dcache oops Oleg Drokin <green@linuxhacker.ru> - 2016-06-04 00:50 +0200
                Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-04 00:50 +0200
                Re: Dcache oops Oleg Drokin <green@linuxhacker.ru> - 2016-06-03 23:20 +0200
                Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-03 23:50 +0200
                Re: Dcache oops Al Viro <viro@ZenIV.linux.org.uk> - 2016-06-04 00:20 +0200

csiph-web