[PATCH 0/2] xfs buf I/O error race test

[PATCH 0/2] xfs buf I/O error race test

[Brian Foster]

Hi all,

Here's a complete test for the XFS buf I/O error race problem reported
by Alex. An incomplete rfc of this test was previously posted here:

http://oss.sgi.com/archives/xfs/2014-08/msg00261.html

This version utilizes the recently posted log recovery delay mechanism.
As such, this test only runs on kernels compiled with XFS debug support.
The first patch updates the generic require sysfs helper to support
global (i.e., non-device specific) sysfs attributes.

Brian

v1:
- Use the log recovery delay mechanism to coordinate I/O failures with
  log recovery.
rfc: http://oss.sgi.com/archives/xfs/2014-08/msg00261.html

Brian Foster (2):
  xfstests/common: don't assume sysfs attrs all reside under test dev
  xfs/051: test buffer use after free race on I/O failure in XFS log
    recovery

 common/rc         |  3 +-
 tests/xfs/011     |  2 +-
 tests/xfs/051     | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/051.out |  2 ++
 tests/xfs/group   |  1 +
 5 files changed, 100 insertions(+), 3 deletions(-)
 create mode 100755 tests/xfs/051
 create mode 100644 tests/xfs/051.out

-- 
1.8.3.1

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

[PATCH 1/2] xfstests/common: don't assume sysfs attrs all reside under test dev

[Brian Foster]

_require_xfs_sysfs() currently assumes that all sysfs attributes reside
under a device-specific subdirectory in the XFS sysfs hierarchy. It is
hardcoded to use the TEST_DEV mount and expect the relative attribute
path as a parameter.

Not all sysfs attributes are associated with specific devices or mount
points, however. Remove the hardcoded device name part of the attribute
path from _require_xfs_sysfs() and let the caller construct the relative
path based on the sysfs XFS root directory.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 common/rc     | 3 +--
 tests/xfs/011 | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/common/rc b/common/rc
index 16da898..01f573e 100644
--- a/common/rc
+++ b/common/rc
@@ -1224,13 +1224,12 @@ _require_xfs_sysfs()
 {
 	attr=$1
 	sysfsdir=/sys/fs/xfs
-	testdev=`_short_dev $TEST_DEV`
 
 	if [ ! -e $sysfsdir ]; then
 		_notrun "no kernel support for XFS sysfs attributes"
 	fi
 
-	if [ ! -z $1 ] && [ ! -e $sysfsdir/$testdev/$attr ]; then
+	if [ ! -z $1 ] && [ ! -e $sysfsdir/$attr ]; then
 		_notrun "sysfs attribute '$attr' is not supported"
 	fi
 }
diff --git a/tests/xfs/011 b/tests/xfs/011
index 658a822..197752c 100755
--- a/tests/xfs/011
+++ b/tests/xfs/011
@@ -85,7 +85,7 @@ _supported_os Linux
 
 _require_scratch
 _require_freeze
-_require_xfs_sysfs log
+_require_xfs_sysfs $(_short_dev $TEST_DEV)/log
 
 rm -f $seqres.full
 
-- 
1.8.3.1

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

[PATCH 2/2] xfs/051: test buffer use after free race on I/O failure in XFS log recovery

[Brian Foster]

A buffer use after free race was discovered in the XFS log recovery
codepath if I/O failures occur during recovery. The I/O submission path
can abort the mount and release the only reference held on some buffers
before I/O completion processing (e.g., async workqueue processing)
might have completed. Badness ensues if the I/O completion path
subsequently attempts to access said buffers.

The test manufactures the race by forcing all writes to fail (via
dm-flakey) after a fixed period of time. A delay is inserted into the
mount codepath to synchronize write failures with log recovery.

Credit for discovery of the race and definition of the reproducible test
case goes to Alex Lyakas.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reported-by: Alex Lyakas <alex@zadarastorage.com>
---
 tests/xfs/051     | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/051.out |  2 ++
 tests/xfs/group   |  1 +
 3 files changed, 98 insertions(+)
 create mode 100755 tests/xfs/051
 create mode 100644 tests/xfs/051.out

diff --git a/tests/xfs/051 b/tests/xfs/051
new file mode 100755
index 0000000..a84746b
--- /dev/null
+++ b/tests/xfs/051
@@ -0,0 +1,95 @@
+#! /bin/bash
+# FS QA Test No. 051
+#
+# Simulate a buffer use after free race in XFS log recovery. The race triggers
+# on I/O failures during log recovery. Note that this test is dangerous as it
+# causes BUG() errors or a panic.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2013 Oracle, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	killall -9 $FSSTRESS_PROG > /dev/null 2>&1
+	_scratch_unmount > /dev/null 2>&1
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/dmflakey
+
+# Modify as appropriate.
+_supported_fs xfs
+_supported_os Linux
+
+_require_scratch
+_require_dm_flakey
+_require_xfs_sysfs debug/log_recovery_delay
+
+echo "Silence is golden."
+
+_scratch_mkfs_xfs >/dev/null 2>&1
+_scratch_mount
+
+# Start a workload and shutdown the fs. The subsequent mount will require log
+# recovery.
+$FSSTRESS_PROG -n 9999 -p 2 -w -d $SCRATCH_MNT > /dev/null 2>&1 &
+sleep 5
+src/godown -f $SCRATCH_MNT
+killall -q $FSSTRESS_PROG
+wait
+_scratch_unmount
+
+# Initialize a dm-flakey device that will pass I/Os for 5s and fail thereafter.
+_init_flakey
+BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
+FLAKEY_TABLE="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 5 180"
+_load_flakey_table $FLAKEY_ALLOW_WRITES
+
+# Set a 10s log recovery delay and mount the flakey device. This should allow
+# initial writes to proceed (e.g., stale log block reset) and then let the
+# flakey uptime timer expire such that I/Os will fail by the time log recovery
+# starts.
+echo 10 > /sys/fs/xfs/debug/log_recovery_delay
+
+# The mount should fail due to dm-flakey. Note that this is dangerous on kernels
+# without the xfs_buf log recovery race fixes.
+_mount_flakey > /dev/null 2>&1
+
+echo 0 > /sys/fs/xfs/debug/log_recovery_delay
+
+_cleanup_flakey
+
+# replay the log
+_scratch_mount
+_scratch_unmount
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/051.out b/tests/xfs/051.out
new file mode 100644
index 0000000..5180bc4
--- /dev/null
+++ b/tests/xfs/051.out
@@ -0,0 +1,2 @@
+QA output created by 051
+Silence is golden.
diff --git a/tests/xfs/group b/tests/xfs/group
index 4d35df5..9784dea 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -47,6 +47,7 @@
 048 other auto quick
 049 rw auto quick
 050 quota auto quick
+051 dangerous
 052 quota db auto quick
 054 quota auto quick
 055 dump ioctl remote tape
-- 
1.8.3.1

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

Re: [PATCH 2/2] xfs/051: test buffer use after free race on I/O failure in XFS log recovery

[Dave Chinner]

On Tue, Sep 02, 2014 at 10:22:41AM -0400, Brian Foster wrote:
> A buffer use after free race was discovered in the XFS log recovery
> codepath if I/O failures occur during recovery. The I/O submission path
> can abort the mount and release the only reference held on some buffers
> before I/O completion processing (e.g., async workqueue processing)
> might have completed. Badness ensues if the I/O completion path
> subsequently attempts to access said buffers.
.....
> 
> case goes to Alex Lyakas.
> index 4d35df5..9784dea 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -47,6 +47,7 @@
>  048 other auto quick
>  049 rw auto quick
>  050 quota auto quick
> +051 dangerous

I'm going to consider this auto/log/metadata rather than dangerous.
Once the bug is fixed, we want to continue running this test as a
regression test, and nobody does that with the dangerous group....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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