| Message ID | 1478166720-14354-1-git-send-email-eguan@redhat.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
On Thu, Nov 03, 2016 at 05:52:00PM +0800, Eryu Guan wrote: > When sb_inopblock is corrupted (out of boundary in this case), XFS > should not crash and refuse to mount. > > Kernel commit 392c6de98af1 ("xfs: sanitize sb_inopblock in > xfs_mount_validate_sb") addresses this issue. Ok, seems like something addressed by the fuzzing tests, but we can ignore that for now. > +_scratch_mkfs >>$seqres.full > + > +# corrupt sb_inopblock > +_scratch_xfs_db -x -c "sb 0" -c "write -c inopblock 512" >>$seqres.full > + > +# mount corrupted fs > +_scratch_mount >>$seqres.full 2>&1 > + > +# no crash, and SCRATCH_DEV should not be mounted either > +_is_mounted $SCRATCH_DEV Rather than test that we caught /one/ corrupt field, why not loop here corrupting each superblock field in turn and checking that the corruption is caught properly? i.e. shouldn't we be proactively testing that we're catching all the obvious corruptions, rather than just testing the out-of-bounds issues that we've already found and fixed? Cheers, Dave.
On Fri, Nov 04, 2016 at 11:54:34AM +1100, Dave Chinner wrote: > On Thu, Nov 03, 2016 at 05:52:00PM +0800, Eryu Guan wrote: > > When sb_inopblock is corrupted (out of boundary in this case), XFS > > should not crash and refuse to mount. > > > > Kernel commit 392c6de98af1 ("xfs: sanitize sb_inopblock in > > xfs_mount_validate_sb") addresses this issue. > > Ok, seems like something addressed by the fuzzing tests, but we can > ignore that for now. > > > +_scratch_mkfs >>$seqres.full > > + > > +# corrupt sb_inopblock > > +_scratch_xfs_db -x -c "sb 0" -c "write -c inopblock 512" >>$seqres.full > > + > > +# mount corrupted fs > > +_scratch_mount >>$seqres.full 2>&1 > > + > > +# no crash, and SCRATCH_DEV should not be mounted either > > +_is_mounted $SCRATCH_DEV > > Rather than test that we caught /one/ corrupt field, why not > loop here corrupting each superblock field in turn and checking > that the corruption is caught properly? > > i.e. shouldn't we be proactively testing that we're catching all the > obvious corruptions, rather than just testing the out-of-bounds > issues that we've already found and fixed? I've been working on xfstests that do exactly that for a while now and am getting ready to do a big patch dump of online scrub/repair + more dangerous_fuzzers tests to see how well the recovery tools work. In the meantime I doubt it hurts to have this regression test while I get my act together. :) --D > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- > 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
diff --git a/tests/xfs/118 b/tests/xfs/118 new file mode 100755 index 0000000..680add8 --- /dev/null +++ b/tests/xfs/118 @@ -0,0 +1,66 @@ +#! /bin/bash +# FS QA Test 118 +# +# Mount an XFS with out of bounds inopblock +# +#----------------------------------------------------------------------- +# Copyright (c) 2016 Red Hat 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.* +} + +# 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 +_supported_fs xfs +_supported_os IRIX Linux +# we corrupt scratch dev on purpose +_require_scratch_nocheck + +_scratch_mkfs >>$seqres.full + +# corrupt sb_inopblock +_scratch_xfs_db -x -c "sb 0" -c "write -c inopblock 512" >>$seqres.full + +# mount corrupted fs +_scratch_mount >>$seqres.full 2>&1 + +# no crash, and SCRATCH_DEV should not be mounted either +_is_mounted $SCRATCH_DEV + +# success, all done +echo "Silence is golden" +status=0 +exit diff --git a/tests/xfs/118.out b/tests/xfs/118.out new file mode 100644 index 0000000..3daed86 --- /dev/null +++ b/tests/xfs/118.out @@ -0,0 +1,2 @@ +QA output created by 118 +Silence is golden diff --git a/tests/xfs/group b/tests/xfs/group index 3296eb9..3bfcb80 100644 --- a/tests/xfs/group +++ b/tests/xfs/group @@ -115,6 +115,7 @@ 115 parent attr 116 quota auto quick 117 fuzzers +118 auto quick metadata 119 log v2log auto freeze dangerous 120 fuzzers 121 log auto quick
When sb_inopblock is corrupted (out of boundary in this case), XFS should not crash and refuse to mount. Kernel commit 392c6de98af1 ("xfs: sanitize sb_inopblock in xfs_mount_validate_sb") addresses this issue. Signed-off-by: Eryu Guan <eguan@redhat.com> --- tests/xfs/118 | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/118.out | 2 ++ tests/xfs/group | 1 + 3 files changed, 69 insertions(+) create mode 100755 tests/xfs/118 create mode 100644 tests/xfs/118.out