Re: XFS hole punch races

From: Dave Chinner <david@fromorbit.com>
To: Ben Hutchings <ben@decadent.org.uk>
Cc: Jan Kara <jack@suse.cz>, stable@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: XFS hole punch races
Date: Sun, 5 Jun 2016 09:28:00 +1000	[thread overview]
Message-ID: <20160604232800.GW26977@dastard> (raw)
In-Reply-To: <1465060270.2847.149.camel@decadent.org.uk>

On Sat, Jun 04, 2016 at 06:11:10PM +0100, Ben Hutchings wrote:
> On Tue, 2016-03-22 at 16:57 +0100, Jan Kara wrote:
> 
> Hi,
> 
> similarly to ext4 also XFS had races between hole punching and page faults
> which could result in data corruption. The fixes were merged in 4.1-rc1 but
> it might make sense to backport them to older stable releases given the
> nature of the issue.
> 
> Relevant fixes are:
> 
> de0e8c20ba3a65b0f15040aabbefdc1999876e6b
> 075a924d45cc69c75a35f20b4912b85aa98b180a
> e8e9ad42c1f1e1bfbe0e8c32c8cac02e9ebfb7ef
> 0f9160b444e4de33b65dfcd3b901358a3129461a
> 723cac48473358939759885a18e8df113ea96138
> ec56b1f1fdc69599963574ce94cc5693d535dd64
> 
> 
> You missed the first in that sequence:
> 
> 653c60b633a9 xfs: introduce mmap/truncate lock
> 
> For 3.16, I've queued up all those fixes with one further prerequisite:
> 
> 812176832169 xfs: fix swapext ilock deadlock
> 
> For 3.2, I've queued up all but 723cac484733, with these additional
> prerequisites:
> 
> f38996f57687 xfs: reduce ilock hold times in xfs_setattr_size
> bc4010ecb8f4 xfs: use iolock on XFS_IOC_ALLOCSP calls
> 76ca4c238cf5 xfs: always take the iolock around xfs_setattr_size
> 5f8aca8b43f4 xfs: always hold the iolock when calling xfs_change_file_space
> I realise I'll need to check for regressions with xfstests.

Hi Ben,

You do realise that this sort of backport effectively makes the
stable kernels unsupportable by the upstream XFS developers? You're
taking random changes from the upstream kernel until the kernel
compiles, and then mostly hoping that it works.

It's trivially easy to break truncate by screwing up the locking and
IO barriers that it depends on, and this set of patches make quite a
lot of different changes that have an unknown set of dependencies
for correct behaviour. xfstests won't find all those problems; at
best it will tell us that there won't be obvious problems.

Stable kernels are supposed to be stable, and backports like this
are riskier than the original changes in the upstream kernels. if a
user is hitting a mmap/hole-punch race on an XFS filesytem (I can't
remember any reports to the upstream list for any kernel) on a 3.2
kernel, then correct answer is "upgrade to a newer upstream kernel",
not "do a risky backport that exposes the entire user base to the
potential of new data corruption bugs"....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs