All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: ocfs2-devel@oss.oracle.com, mark@fasheh.com, jlbec@evilplan.org,
	linux-fsdevel@vger.kernel.org, Junxiao Bi <junxiao.bi@oracle.com>,
	Joseph Qi <joseph.qi@huawei.com>,
	Mark Fasheh <mfasheh@versity.com>
Subject: Re: [PATCH] ocfs2: fix inode bh swapping mixup in ocfs2_reflink_inodes_lock
Date: Wed, 13 Mar 2019 09:47:40 -0700	[thread overview]
Message-ID: <20190313164740.GB4657@magnolia> (raw)
In-Reply-To: <20190313093734.b924a6a8f07afd277ab96e14@linux-foundation.org>

On Wed, Mar 13, 2019 at 09:37:34AM -0700, Andrew Morton wrote:
> On Tue, 12 Mar 2019 14:49:10 -0700 "Darrick J. Wong" <darrick.wong@oracle.com> wrote:
> 
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > ocfs2_reflink_inodes_lock can swap the inode1/inode2 variables so that
> > we always grab cluster locks in order of increasing inode number.
> > Unfortunately, we forget to swap the inode record buffer head pointers
> > when we've done this, which leads to incorrect bookkeepping when we're
> > trying to make the two inodes have the same refcount tree.
> > 
> > This has the effect of causing filesystem shutdowns if you're trying to
> > reflink data from inode 100 into inode 97, where inode 100 already has a
> > refcount tree attached and inode 97 doesn't.  The reflink code decides
> > to copy the refcount tree pointer from 100 to 97, but uses inode 97's
> > inode record to open the tree root (which it doesn't have) and blows up.
> > This issue causes filesystem shutdowns and metadata corruption!
> 
> Sounds serious.
> 
> > Fixes: 29ac8e856cb369 ("ocfs2: implement the VFS clone_range, copy_range, and dedupe_range features")]
> 
> November 2016.  Should we be adding cc:stable?

Yeah.  I sent along an RFC version of the testcase (generic/94[134])
that hit this bug now that I've been able to get an overnight testing
run completed with the new tests on the other filesystems.

--D

> Folks, could we please get prompt review of this one?
> 
> > mark@fasheh.com
> 
> hm, I have mfasheh@versity.com but MAINTAINERS says mark@fasheh.com. 
> Mark, can you please clarify?

WARNING: multiple messages have this Message-ID (diff)
From: Darrick J. Wong <darrick.wong@oracle.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: ocfs2-devel@oss.oracle.com, mark@fasheh.com, jlbec@evilplan.org,
	linux-fsdevel@vger.kernel.org, Junxiao Bi <junxiao.bi@oracle.com>,
	Joseph Qi <joseph.qi@huawei.com>,
	Mark Fasheh <mfasheh@versity.com>
Subject: [Ocfs2-devel] [PATCH] ocfs2: fix inode bh swapping mixup in ocfs2_reflink_inodes_lock
Date: Wed, 13 Mar 2019 09:47:40 -0700	[thread overview]
Message-ID: <20190313164740.GB4657@magnolia> (raw)
In-Reply-To: <20190313093734.b924a6a8f07afd277ab96e14@linux-foundation.org>

On Wed, Mar 13, 2019 at 09:37:34AM -0700, Andrew Morton wrote:
> On Tue, 12 Mar 2019 14:49:10 -0700 "Darrick J. Wong" <darrick.wong@oracle.com> wrote:
> 
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > ocfs2_reflink_inodes_lock can swap the inode1/inode2 variables so that
> > we always grab cluster locks in order of increasing inode number.
> > Unfortunately, we forget to swap the inode record buffer head pointers
> > when we've done this, which leads to incorrect bookkeepping when we're
> > trying to make the two inodes have the same refcount tree.
> > 
> > This has the effect of causing filesystem shutdowns if you're trying to
> > reflink data from inode 100 into inode 97, where inode 100 already has a
> > refcount tree attached and inode 97 doesn't.  The reflink code decides
> > to copy the refcount tree pointer from 100 to 97, but uses inode 97's
> > inode record to open the tree root (which it doesn't have) and blows up.
> > This issue causes filesystem shutdowns and metadata corruption!
> 
> Sounds serious.
> 
> > Fixes: 29ac8e856cb369 ("ocfs2: implement the VFS clone_range, copy_range, and dedupe_range features")]
> 
> November 2016.  Should we be adding cc:stable?

Yeah.  I sent along an RFC version of the testcase (generic/94[134])
that hit this bug now that I've been able to get an overnight testing
run completed with the new tests on the other filesystems.

--D

> Folks, could we please get prompt review of this one?
> 
> > mark at fasheh.com
> 
> hm, I have mfasheh at versity.com but MAINTAINERS says mark at fasheh.com. 
> Mark, can you please clarify?

  reply	other threads:[~2019-03-13 16:48 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-12 21:49 [PATCH] ocfs2: fix inode bh swapping mixup in ocfs2_reflink_inodes_lock Darrick J. Wong
2019-03-12 21:49 ` [Ocfs2-devel] " Darrick J. Wong
2019-03-13 16:37 ` Andrew Morton
2019-03-13 16:37   ` [Ocfs2-devel] " Andrew Morton
2019-03-13 16:47   ` Darrick J. Wong [this message]
2019-03-13 16:47     ` Darrick J. Wong
2019-03-13 16:46 ` [RFC PATCH] clonerange: test remapping the rainbow Darrick J. Wong
2019-03-13 16:46   ` [Ocfs2-devel] " Darrick J. Wong
2019-03-23  7:14   ` Eryu Guan
2019-03-25 23:49     ` Darrick J. Wong
2019-03-25 23:49       ` [Ocfs2-devel] " Darrick J. Wong
2019-03-14  1:06 ` [Ocfs2-devel] [PATCH] ocfs2: fix inode bh swapping mixup in ocfs2_reflink_inodes_lock Joseph Qi
2019-03-14  1:06   ` Joseph Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190313164740.GB4657@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=jlbec@evilplan.org \
    --cc=joseph.qi@huawei.com \
    --cc=junxiao.bi@oracle.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mark@fasheh.com \
    --cc=mfasheh@versity.com \
    --cc=ocfs2-devel@oss.oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.