From mboxrd@z Thu Jan  1 00:00:00 1970
From: Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: panic in do_last()
Date: Fri, 18 Apr 2014 03:38:22 +0100
Message-ID: <20140418023822.GI18016@ZenIV.linux.org.uk>
References: <CAF1ivSYoOvxMg3WjQYH5Jtv3tTY8NUn+QM7SvQWHJ1M1bVCZeg@mail.gmail.com>
 <20140418015750.GG18016@ZenIV.linux.org.uk>
 <CAF1ivSbqM2LorrED3MvgKBDxA9sLz_RkOEPwAfvYYmL5C9K0vA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: David Howells <dhowells@redhat.com>, linux-fsdevel@vger.kernel.org,
	Hugh Dickins <hughd@google.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
To: Lin Ming <minggr@gmail.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:34744 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751041AbaDRCiZ (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Thu, 17 Apr 2014 22:38:25 -0400
Content-Disposition: inline
In-Reply-To: <CAF1ivSbqM2LorrED3MvgKBDxA9sLz_RkOEPwAfvYYmL5C9K0vA@mail.gmail.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Thu, Apr 17, 2014 at 07:16:30PM -0700, Lin Ming wrote:
> (gdb) list *do_last+0x7d2
> 0xffffffff811095a5 is in do_last (/home/mlin/linux/fs/namei.c:3036).
> 3031 if ((open_flag & O_CREAT) && d_is_dir(nd->path.dentry))
> 3032            goto out;
> 3033 error = -ENOTDIR;
> 3034 if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
> 3035            goto out;
> 3036 if (!S_ISREG(nd->inode->i_mode))
> 3037            will_truncate = false;

Aha, so my guess in another posting was correct.  OK...

So it's not about RCU case at all - if dentry changes under us
in lazy mode, we'll simply fail when we get to checking d_seq.  It's
non-lazy case + transition from negative to positive that causes the
trouble.

Another piece of breakage in there is should_follow_link() directly
afterwards - there the problem is with false negatives.  I.e. we
see new dentry->d_inode, but not the new dentry->d_flags.

I wonder if the right fix would be simply
	if (!inode || d_is_negative(dentry))
bailing out unless both updates are seen.  Probably cheaper than any
games with barriers...

Comments?