From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: fs/namei.c: Misuse of sequence counts? Date: Sun, 12 Oct 2014 00:46:35 +0100 Message-ID: <20141011234635.GL7996@ZenIV.linux.org.uk> References: <20141011225808.GA20777@zzz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Eric Biggers Return-path: Content-Disposition: inline In-Reply-To: <20141011225808.GA20777@zzz> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Sat, Oct 11, 2014 at 05:58:08PM -0500, Eric Biggers wrote: > In follow_dotdot_rcu(), said commit moved loads of the inode to just before > read_seqcount_begin(), in several instances. I don't think this is correct, > because (as I understand it) read_seqcount_begin() is opening a seq-read > critical section on the new dentry. So the inode load should come *after* it, > as in the original, to ensure the inode pointer is correctly matched with the > sequence count. Nope. What we do is * pick parent inode and seqcount (in whatever order) * THEN check that child is still unchanged. The second part guarantees that parent dentry had been the parent of child all along, since the moment we'd first fetched _child's_ seqcount. And since a pinned positive dentry can't have its ->d_inode changed, we know that the value of parent's inode we'd fetched remained valid at least until we'd checked the child's seqcount and found it unchanged. Which means that we had it valid at some point after we'd fetched parent's seqcount. The crucial part is that dentry cannot change its ->d_inode for as long as there are references to it.