Discussion:
[PATCH 1/2] xfstests/common: don't assume sysfs attrs all reside under test dev
Brian Foster
2014-09-02 14:22:40 UTC
Permalink
_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 <***@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
Brian Foster
2014-09-02 14:22:41 UTC
Permalink
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 <***@redhat.com>
Reported-by: Alex Lyakas <***@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
Dave Chinner
2014-09-08 10:47:49 UTC
Permalink
Post by 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.
.....
Post by Brian Foster
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
***@fromorbit.com
Loading...