Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

From: Linus Torvalds <torvalds@linux-foundation.org>
To: NeilBrown <neilb@suse.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()
Date: Thu, 9 Nov 2017 12:23:19 -0800	[thread overview]
Message-ID: <CA+55aFxaoyND1DDk5HqUw97i6-+uMRBbCx4mo=yTKYnBnO2owg@mail.gmail.com> (raw)
In-Reply-To: <151019772763.30101.16040338743875884111.stgit@noble>

On Wed, Nov 8, 2017 at 7:22 PM, NeilBrown <neilb@suse.com> wrote:
> d_move() will call __d_drop() and then __d_rehash()
> on the dentry being moved.  This creates a small window
> when the dentry appears to be unhashed.  Many tests
> of d_unhashed() are made under ->d_lock and so are safe
> from racing with this window, but some aren't.
> In particular, getcwd() calls d_unlinked() (which calls
> d_unhashed()) without d_lock protection, so it can race.

Hmm.

I see what you're doing, but I don't necessarily agree.

I would actually almost prefer that we simply change __d_move() itself.

The problem is that __d_move() really wants to move the hashes things
atomically, but instead of doing that it does a "unhash and then
rehash".

How nasty would it be to just expand the calls to __d_drop/__d_rehash
into __d_move itself, and take both has list locks at the same time
(with the usual ordering and checking if it's the same list, of
course).

                     Linus