From mboxrd@z Thu Jan  1 00:00:00 1970
From: Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: Intentionally corrupted vfat fs causing BUG
Date: Thu, 23 Oct 2014 17:01:06 +0100
Message-ID: <20141023160106.GB7996@ZenIV.linux.org.uk>
References: <20141010205706.GJ27150@sli.dy.fi>
 <87h9z97aoh.fsf@devron.myhome.or.jp>
 <CAFLxGvxiP8mUK4Kjm1p9m5J=d5qNpcpXuJddkRVFkc++BFFnFg@mail.gmail.com>
 <8761fo7667.fsf@devron.myhome.or.jp>
 <543B8BC7.1040501@nod.at>
 <87y4sk5pul.fsf@devron.myhome.or.jp>
 <543B8FA7.9000106@nod.at>
 <87r3yc5oqt.fsf@devron.myhome.or.jp>
 <5443E87A.2060207@nod.at>
 <87oat29551.fsf@devron.myhome.or.jp>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Richard Weinberger <richard@nod.at>,
	Sami Liedes <sami.liedes@iki.fi>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
To: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:37353 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1755527AbaJWQBM (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Thu, 23 Oct 2014 12:01:12 -0400
Content-Disposition: inline
In-Reply-To: <87oat29551.fsf@devron.myhome.or.jp>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Fri, Oct 24, 2014 at 12:28:58AM +0900, OGAWA Hirofumi wrote:
> > What about this one?
> 
> Looks like strange. If we want to tackle this at per-FS. We should not
> return double linked dir at first.  Since double linked breaks dir
> hierarchy, even if this one can avoid that Oops, double linked can be
> easily the cause of another Oops, deadlock, etc.
> 
> Well, this patch is untested though. For example, somethings like
> following. But, again, this fixes only one of cases in double linked.
> (And to fix fully, my mind was already talked.)

Hmm...  Why hadn't d_splice_alias() caught that, though?  Look: in that
case we see that inode is non-NULL, a directory and has an alias
(namely, dentry->d_parent).  So we hit this:
                new = __d_find_any_alias(inode);
                if (new) {
                        if (!IS_ROOT(new)) {
                                spin_unlock(&inode->i_lock);
                                dput(new);
                                return ERR_PTR(-EIO);
                        }
                        if (d_ancestor(new, dentry)) {
                                spin_unlock(&inode->i_lock);
                                dput(new);
                                return ERR_PTR(-EIO);
                        }
and depending on whether that ->d_parent had been the filesystem root,
we hit either the former or the latter.  IOW, we should've done
exactly that...

FWIW, there *is* a bug in that path - we ought to have done iput(inode)
on both failure exits in order to follow the calling conventions.  But that
doesn't look like it would oops right there...

Could somebody repost the oops stack trace?  The bug in d_splice_alias()
is real (and fairly old), but I'd like to understand if there's anything
else in the game...