Re: [PATCH RFC] NFSD: fix cannot umounting mount points under pseudo root

From: NeilBrown <neilb@suse.de>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	Kinglong Mee <kinglongmee@gmail.com>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH RFC] NFSD: fix cannot umounting mount points under pseudo root
Date: Fri, 1 May 2015 13:08:26 +1000	[thread overview]
Message-ID: <20150501130826.40721dd0@notabene.brown> (raw)
In-Reply-To: <20150501022939.GQ889@ZenIV.linux.org.uk>

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

On Fri, 1 May 2015 03:29:40 +0100 Al Viro <viro@ZenIV.linux.org.uk> wrote:

> On Fri, May 01, 2015 at 12:23:33PM +1000, NeilBrown wrote:
> > > What kind of consistency warranties do callers expect, BTW?  You do realize
> > > that between iterate_dir() and callbacks an entry might have been removed
> > > and/or replaced?
> > 
> > For READDIR_PLUS, lookup_one_len is called on each name and it requires
> > i_mutex, so the code currently holds i_mutex over the whole sequence.
> > This is triggering a deadlock.
> 
> Yes, I've seen the context.  However, you are _not_ holding it between
> actual iterate_dir() and those callbacks, which opens a window when
> directory might have been changed.
> 
> Again, what kind of consistency is expected by callers?  Are they ready to
> cope with "there's no such entry anymore" or "inumber is nothing like
> what we'd put in ->ino, since it's no the same object" or "->d_type is
> completely unrelated to what we'd found, since the damn thing had been
> removed and created from scratch"?

Ah, sorry.

Yes, the callers are prepared for "there's no such entry anymore".
They don't use d_type, so don't care if it might be meaningless.
NFSv4 doesn't use ino either, but NFSv3 does and isn't properly cautious
about ino changing.

In nfs3xdr, we should probably pass 'ino' to encode_entryplus_baggage() and
thence to compose_entry_fh() and it should report failure if
dchild->d_inode->i_ino doesn't match.
Simply not returning the extra attributes is perfectly acceptable in NFSv3.

So it looks like we are mostly OK here - we don't really need i_mutex to be
held for very long.

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]