From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:44741 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S933731AbcCJC7y (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Wed, 9 Mar 2016 21:59:54 -0500
Date: Thu, 10 Mar 2016 02:59:49 +0000
From: Al Viro <viro@ZenIV.linux.org.uk>
To: "Drokin, Oleg" <oleg.drokin@intel.com>
Cc: "Dilger, Andreas" <andreas.dilger@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"<linux-fsdevel@vger.kernel.org>" <linux-fsdevel@vger.kernel.org>,
	Theodore Ts'o <tytso@mit.edu>, Mark Fasheh <mfasheh@suse.com>
Subject: Re: races in ll_splice_alias() and elsewhere (ext4, ocfs2)
Message-ID: <20160310025948.GG17997@ZenIV.linux.org.uk>
References: <20160308160537.GV17997@ZenIV.linux.org.uk>
 <498D5A19-9E55-48D7-B5CF-34CA5769FF7F@intel.com>
 <20160308211148.GX17997@ZenIV.linux.org.uk>
 <A5CA4682-77B1-4DF4-A784-7C7C2FAFC9AF@intel.com>
 <20160309003416.GY17997@ZenIV.linux.org.uk>
 <7C3EBB6F-54AC-4744-BEC1-33EA82216F85@intel.com>
 <20160309012658.GZ17997@ZenIV.linux.org.uk>
 <34C2B1C3-2B7F-490B-A03A-3BCDDFC8BE48@intel.com>
 <20160310022041.GF17997@ZenIV.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160310022041.GF17997@ZenIV.linux.org.uk>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Thu, Mar 10, 2016 at 02:20:42AM +0000, Al Viro wrote:

> Umm...  AFAICS, ext4_d_revalidate() is racy, starting with the very
> first line.  What's to prevent it being moved while we are calling that?
> Lose timeslice on preemption, have mv(1) move it elsewhere, followed by
> rmdir taking the now-empty parent out.  Come back and dir points to
> freed memory, with ci being complete junk.  Looks like oopsen galore...
> Ted, am I missing something subtle here?

BTW, the fact that original parent dentry is pinned by caller doesn't help
at all - by the time we get to ext4_d_revalidate() its ->d_parent might have
been pointing to something we are *not* pinning, with another rename() +
rmdir() completing the problem.  It's going to be hard to hit, but not
impossible.  Have d_move() happen right after we'd found the match in
__d_lookup(), then get preempted just as we'd fetched (already changed)
->d_parent->d_inode in ext4_d_revalidate().  The second rename() + rmdir()
have to complete by the time we regain CPU and we are screwed.