Thread (6 messages) 6 messages, 3 authors, 2017-09-21

Re: [PATCH] xfs: Add test for CVE-2017-14340

From: Eryu Guan <hidden>
Date: 2017-09-21 05:56:59
Also in: fstests

On Wed, Sep 20, 2017 at 03:40:06AM +0000, Richard Wareing wrote:
Darrick J. Wong [off-list ref] wrote:
quoted
On Tue, Sep 19, 2017 at 05:30:54PM -0700, Richard Wareing wrote:
quoted
Verify kernel doesn't panic when user attempts to set realtime flags
on non-realtime FS, using kernel compiled with CONFIG_XFS_RT.  Unpatched
kernels will panic during this test.  Kernels not compiled with
CONFIG_XFS_RT should pass test.

This bug was fixed via commit b31ff3cdf540110da4572e3e29bd172087af65cc
Oooh, a commit id, nice! :)
quoted
on the main kernel tree.

Signed-off-by: Richard Wareing <redacted>
---
 tests/xfs/431     | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/431.out |  3 ++
 tests/xfs/group   |  1 +
 3 files changed, 90 insertions(+)
 create mode 100755 tests/xfs/431
 create mode 100644 tests/xfs/431.out
diff --git a/tests/xfs/431 b/tests/xfs/431
new file mode 100755
index 0000000..f928abc
--- /dev/null
+++ b/tests/xfs/431
@@ -0,0 +1,86 @@
+#! /bin/bash
+# FS QA Test 431
+#
+# Verify kernel doesn't panic when user attempts to set realtime flags
+# on non-realtime FS, using kernel compiled with CONFIG_XFS_RT.   
Unpatched
+# kernels will panic during this test.  Kernels not compiled with
+# CONFIG_XFS_RT should pass test.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 YOUR NAME HERE.  All Rights Reserved.
Mr. HERE, please update this line! :)
Fixed in v2.
quoted
quoted
+#
+# 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.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs xfs
+_supported_os Linux
+_require_xfs_io_command "chattr"
+_require_xfs_io_command "fsync"
+_require_xfs_io_command "pwrite"
+_require_scratch
+_require_test
+
Shouldn't we unset USE_EXTERNAL/SCRATCH_RTDEV here since the bug only
affects non-rt xfses?
I tried playing around with this, but wasn't able to get it to
work.  The best I think to do is create a "_require_no_realtime"
and skip the test if it's being run under a realtime test run.
Hmm, I don't think we need to exclude test with rtdev, it's true that
the bug only affects non-rt XFS but there's no harm to do such a
'sanity' test with rtdev present.
quoted
quoted
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount
+
+# Set realtime inherit flag on scratch mount, suppress output
+# as this may simply error out on future kernels, we will check
+# exit code instead.
+$XFS_IO_PROG -c 'chattr +t' $SCRATCH_MNT &> /dev/null
+chattr_ret=$?
It seems that xfs_io always returns 0 even the command fails, e.g.

[root@bootp-73-5-205 xfstests]# xfs_io -c "pwrite 0 4k" /
pwrite: Bad file descriptor
[root@bootp-73-5-205 xfstests]# echo $?
0

So we should not depend on xfs_io's return value. And I think doing the
'pwrite' & 'fsync' test unconditionally should be fine, e.g.

$XFS_IO_PROG -c 'chattr +t' $SCRATCH_MNT >>$seqres.full 2>&1
$XFS_IO_PROG -fc 'pwrite 0 1m' -c fsync ..
$XFS_IO_PROG -c 'chattr -t' $SCRATCH_MNT...

I tested on 4.14-rc1 kernels with and without CONFIG_XFS_RT, and with
and without rtdev present when CONFIG_XFS_RT=y, all tests passed.
quoted
quoted
+
+# Erroring out here is fine, this would be desired behavior for
+# FSes without realtime devices present.
+if (( chattr_ret == 0)); then
+  # Attempt to write/fsync data to file
+  $XFS_IO_PROG -fc 'pwrite 0 1m' -c fsync $SCRATCH_MNT/testfile |
+    tee -a $seqres.full | common_line_filter | _filter_xfs_io
(common_line_filter | _filter_xfs_io) == _filter_xfs_io_unique

But I think a bare _filter_xfs_io should just work, there's no common
lines to be filtered anyway.
quoted
quoted
+
+  # Remove the rt inherit flag after we are done or xfs_repair
+  # will fail.
+  $XFS_IO_PROG -c 'chattr -t' $SCRATCH_MNT | tee -a $seqres.full 2>&1
And please use real tab as indention :)
quoted
quoted
+fi
+
+
+rm -f $SCRATCH_MNT/testfile
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/431.out b/tests/xfs/431.out
new file mode 100644
index 0000000..8c14f11
--- /dev/null
+++ b/tests/xfs/431.out
@@ -0,0 +1,3 @@
+QA output created by 431
+wrote 1048576/1048576 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/xfs/group b/tests/xfs/group
index 0a449b9..3f97f02 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -427,3 +427,4 @@
 428 dangerous_fuzzers dangerous_scrub dangerous_online_repair
 429 dangerous_fuzzers dangerous_scrub dangerous_repair
 430 dangerous_fuzzers dangerous_scrub dangerous_online_repair
+431 quick
This ought to have 'dangerous' too, so that everyone knows that it can
crash an unpatched kernel.

(After stable/distros have had a few weeks to fix this you could add it
to the 'auto' group as well so that everyone will run it.)
'quick' is just a subset of 'auto', all 'quick' tests should be in
'auto' group too, the only difference is 'quick' is quick :) usually run
time is less than 30s. If you want to exclude tests in 'dangerous' group
you can run check with '-x dangerous' option.

Thanks,
Eryu
quoted
Fixed in v3.
quoted
--D
quoted
-- 
2.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help