linux-kernel - Re: [PATCH 02/11] vfs: More precise tests in d

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]

Message-ID: <87a9ds55av.fsf@xmission.com>
Date:	Sat, 15 Feb 2014 15:23:20 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Al Viro <viro@...iv.linux.org.uk>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
	Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andy Lutomirski <luto@...capital.net>,
	Rob Landley <rob@...dley.net>,
	Miklos Szeredi <miklos@...redi.hu>,
	Christoph Hellwig <hch@...radead.org>,
	Karel Zak <kzak@...hat.com>,
	"J. Bruce Fields" <bfields@...ldses.org>
Subject: Re: [PATCH 02/11] vfs: More precise tests in d_invalidate

Linus Torvalds <torvalds@...ux-foundation.org> writes:

> On Sat, Feb 15, 2014 at 2:51 PM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> the whole check_submounts_and_drop thing walks the parent chain and
>> locks each parent with the renamelock held for writing.
>
> Oops, my bad about the write lock, brainfart due to grepping and
> reading the wrong context...
>
> check_submounts_and_drop() doesn't do the parent walk with the rename
> lock held for writing, it just holds it for reading.
>
> But it does do that very complex "walk parents and check all siblings"
> and locks them, so the rest of the commentary was correct.

Except that today d_invalidate drops the dcache lock and
calls shrink_dcache_parent.  Which gets you into exactly the same
complex "walk parents and check all siblings" code.

The only difference between the shrink_dcache_parent and
check_submounts_and_drop (not counting the final drop)
is that check_submounts_and_drop aborts when it encounters a dentry
with d_mountpoint set.

So no I am not trying to hide something.  I called out that I changed
this logic in particular and this particular patch all I am doing is
killing the enforcing of 2.2 era logic.  Further I front loaded this
change so I bisect could point it's fingers at this before any other
substantial changes were made if this is indeed a problem.

Beyond that check_submounts_and_drop is what well maintained distributed
filesystems are calling from d_revalidate.

Now I would not be surprised if this change to d_invalidate is a
challenge to get your head around.  It took me a while of reading the
code to realize (a) how the code makes some degree of sense today,
and (b) that the change is semantically safe.

But when shrink_dcache_parent and check_submounts_and_drop are
effectiely the same function I can't possibly see how you can argue how
the locking has changed or that I am trying to hide things.

Eric
--
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/