From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Eryu Guan <eguan@redhat.com>
Cc: linux-xfs@vger.kernel.org, fstests@vger.kernel.org
Subject: Re: [PATCH 7/7] xfs/ext4: check negative inode size
Date: Mon, 9 Jan 2017 12:36:26 -0800 [thread overview]
Message-ID: <20170109203626.GB14033@birch.djwong.org> (raw)
In-Reply-To: <20170109093635.GC1859@eguan.usersys.redhat.com>
On Mon, Jan 09, 2017 at 05:36:35PM +0800, Eryu Guan wrote:
> On Wed, Jan 04, 2017 at 05:05:20PM -0800, Darrick J. Wong wrote:
> > Craft a malicious filesystem image with a negative inode size,
> > then try to trigger a kernel DoS by appending data to the file.
> > Ideally this should trigger verifier errors instead of hanging.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > tests/shared/400 | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/shared/400.out | 5 +++
> > tests/shared/401 | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/shared/401.out | 5 +++
> > tests/shared/group | 2 +
> > tests/xfs/400 | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/xfs/400.out | 5 +++
> > tests/xfs/401 | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/xfs/401.out | 5 +++
> > tests/xfs/group | 2 +
> > 10 files changed, 310 insertions(+)
> > create mode 100755 tests/shared/400
> > create mode 100644 tests/shared/400.out
> > create mode 100755 tests/shared/401
> > create mode 100644 tests/shared/401.out
> > create mode 100755 tests/xfs/400
> > create mode 100644 tests/xfs/400.out
> > create mode 100755 tests/xfs/401
> > create mode 100644 tests/xfs/401.out
> >
> >
> > diff --git a/tests/shared/400 b/tests/shared/400
> > new file mode 100755
> > index 0000000..ba7bcda
> > --- /dev/null
> > +++ b/tests/shared/400
> > @@ -0,0 +1,71 @@
> > +#! /bin/bash
> > +# FSQA Test No. 400
> > +#
> > +# Since loff_t is a signed type, it is invalid for a filesystem to load
> > +# an inode with i_size = -1ULL. Unfortunately, nobody checks this,
> > +# which means that we can trivially DoS the VFS by creating such a file
> > +# and appending to it. This causes an integer overflow in the routines
> > +# underlying writeback, which results in the kernel locking up.
>
> How about adding some more descriptions here? Stating that this test is
> testing buffered I/O, and adding some comments about testing direct I/O
> to shared/401? The same to xfs/400 and xfs/401. Otherwise they look the
> same just from the test description.
Ok.
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2017 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"
> > +
> > +PIDS=""
> > +tmp=/tmp/$$
> > +status=1 # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_os Linux
> > +_supported_fs ext2 ext3 ext4
> > +_require_scratch_nocheck
> > +_disable_dmesg_check
>
> _require_command "$DEBUGFS_PROG" debugfs and..
<nod>
>
> > +
> > +rm -f $seqres.full
> > +
> > +echo "Format and mount"
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +_scratch_mount
> > +
> > +testdir=$SCRATCH_MNT
> > +echo m > $testdir/a
> > +
> > +echo "Corrupt filesystem"
> > +_scratch_unmount
> > +debugfs -w -R "sif /a size -1" $SCRATCH_DEV >> $seqres.full 2>&1
>
> $DEBUGFS_PROG here
<nod>
>
> > +
> > +echo "Remount, try to append"
> > +_scratch_mount
> > +dd if=/dev/zero of=$testdir/a bs=512 count=1 oflag=append conv=notrunc >> $seqres.full 2>&1 || echo "Write did not succeed (ok)."
> > +sync
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/shared/400.out b/tests/shared/400.out
> > new file mode 100644
> > index 0000000..ddf8a28
> > --- /dev/null
> > +++ b/tests/shared/400.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 400
> > +Format and mount
> > +Corrupt filesystem
> > +Remount, try to append
> > +Write did not succeed (ok).
> > diff --git a/tests/shared/401 b/tests/shared/401
> > new file mode 100755
> > index 0000000..d790381
> > --- /dev/null
> > +++ b/tests/shared/401
> > @@ -0,0 +1,71 @@
> > +#! /bin/bash
> > +# FSQA Test No. 401
> > +#
> > +# Since loff_t is a signed type, it is invalid for a filesystem to load
> > +# an inode with i_size = -1ULL. Unfortunately, nobody checks this,
> > +# which means that we can trivially DoS the VFS by creating such a file
> > +# and appending to it. This causes an integer overflow in the routines
> > +# underlying writeback, which results in the kernel locking up.
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2017 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"
> > +
> > +PIDS=""
> > +tmp=/tmp/$$
> > +status=1 # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_os Linux
> > +_supported_fs ext2 ext3 ext4
> > +_require_scratch_nocheck
> > +_disable_dmesg_check
> > +
> > +rm -f $seqres.full
> > +
> > +echo "Format and mount"
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +_scratch_mount
> > +
> > +testdir=$SCRATCH_MNT
> > +echo m > $testdir/a
> > +
> > +echo "Corrupt filesystem"
> > +_scratch_unmount
> > +debugfs -w -R "sif /a size 0xFFFFFFFFFFFFFE00" $SCRATCH_DEV >> $seqres.full 2>&1
>
> Better to have your explanation to 0XFFFFFFFFFFFFFE00 as comments. (Same
> to xfs/400 and xfs/401.)
>
> "The 0xFFFFFFFFFFFFFE00 rounds the file size down to a multiple of 512
> so that we can do the directio"
Ok, fixed.
--D
>
> Thanks,
> Eryu
>
> > +
> > +echo "Remount, try to append"
> > +_scratch_mount
> > +dd if=/dev/zero of=$testdir/a bs=512 count=1 oflag=direct,append conv=notrunc >> $seqres.full 2>&1 || echo "Write did not succeed (ok)."
> > +sync
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/shared/401.out b/tests/shared/401.out
> > new file mode 100644
> > index 0000000..0d45add
> > --- /dev/null
> > +++ b/tests/shared/401.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 401
> > +Format and mount
> > +Corrupt filesystem
> > +Remount, try to append
> > +Write did not succeed (ok).
> > diff --git a/tests/shared/group b/tests/shared/group
> > index 55bb594..64d2d2f 100644
> > --- a/tests/shared/group
> > +++ b/tests/shared/group
> > @@ -13,3 +13,5 @@
> > 272 auto enospc rw
> > 289 auto quick
> > 298 auto trim
> > +400 dangerous_fuzzers
> > +401 dangerous_fuzzers
> > diff --git a/tests/xfs/400 b/tests/xfs/400
> > new file mode 100755
> > index 0000000..eed8fdf
> > --- /dev/null
> > +++ b/tests/xfs/400
> > @@ -0,0 +1,72 @@
> > +#! /bin/bash
> > +# FSQA Test No. 400
> > +#
> > +# Since loff_t is a signed type, it is invalid for a filesystem to load
> > +# an inode with i_size = -1ULL. Unfortunately, nobody checks this,
> > +# which means that we can trivially DoS the VFS by creating such a file
> > +# and appending to it. This causes an integer overflow in the routines
> > +# underlying writeback, which results in the kernel locking up.
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2017 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"
> > +
> > +PIDS=""
> > +tmp=/tmp/$$
> > +status=1 # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_os Linux
> > +_supported_fs xfs
> > +_require_scratch_nocheck
> > +_disable_dmesg_check
> > +
> > +rm -f $seqres.full
> > +
> > +echo "Format and mount"
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +_scratch_mount
> > +
> > +testdir=$SCRATCH_MNT
> > +echo m > $testdir/a
> > +inum=$(stat -c "%i" $testdir/a)
> > +
> > +echo "Corrupt filesystem"
> > +_scratch_unmount
> > +_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full
> > +
> > +echo "Remount, try to append"
> > +_scratch_mount
> > +dd if=/dev/zero of=$testdir/a bs=512 count=1 oflag=append conv=notrunc >> $seqres.full 2>&1 || echo "Write did not succeed (ok)."
> > +sync
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/400.out b/tests/xfs/400.out
> > new file mode 100644
> > index 0000000..ddf8a28
> > --- /dev/null
> > +++ b/tests/xfs/400.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 400
> > +Format and mount
> > +Corrupt filesystem
> > +Remount, try to append
> > +Write did not succeed (ok).
> > diff --git a/tests/xfs/401 b/tests/xfs/401
> > new file mode 100755
> > index 0000000..2be684a
> > --- /dev/null
> > +++ b/tests/xfs/401
> > @@ -0,0 +1,72 @@
> > +#! /bin/bash
> > +# FSQA Test No. 401
> > +#
> > +# Since loff_t is a signed type, it is invalid for a filesystem to load
> > +# an inode with i_size = -1ULL. Unfortunately, nobody checks this,
> > +# which means that we can trivially DoS the VFS by creating such a file
> > +# and appending to it. This causes an integer overflow in the routines
> > +# underlying writeback, which results in the kernel locking up.
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2017 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"
> > +
> > +PIDS=""
> > +tmp=/tmp/$$
> > +status=1 # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_os Linux
> > +_supported_fs xfs
> > +_require_scratch_nocheck
> > +_disable_dmesg_check
> > +
> > +rm -f $seqres.full
> > +
> > +echo "Format and mount"
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +_scratch_mount
> > +
> > +testdir=$SCRATCH_MNT
> > +echo m > $testdir/a
> > +inum=$(stat -c "%i" $testdir/a)
> > +
> > +echo "Corrupt filesystem"
> > +_scratch_unmount
> > +_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -512' >> $seqres.full
> > +
> > +echo "Remount, try to append"
> > +_scratch_mount
> > +dd if=/dev/zero of=$testdir/a bs=512 count=1 oflag=direct,append conv=notrunc >> $seqres.full 2>&1 || echo "Write did not succeed (ok)."
> > +sync
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/401.out b/tests/xfs/401.out
> > new file mode 100644
> > index 0000000..0d45add
> > --- /dev/null
> > +++ b/tests/xfs/401.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 401
> > +Format and mount
> > +Corrupt filesystem
> > +Remount, try to append
> > +Write did not succeed (ok).
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index c237b50..10ba27b 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -334,3 +334,5 @@
> > 345 auto quick clone
> > 346 auto quick clone
> > 347 auto quick clone
> > +400 dangerous_fuzzers
> > +401 dangerous_fuzzers
> >
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2017-01-09 20:36 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-05 1:04 [PATCH 0/7] xfstests: misc reflink test fixes Darrick J. Wong
2017-01-05 1:04 ` [PATCH 1/7] ocfs2: test reflinking to inline data files Darrick J. Wong
2017-01-05 1:04 ` [PATCH 2/7] common: add leading underscore to get_block_size Darrick J. Wong
2017-01-09 10:20 ` Eryu Guan
2017-01-09 21:32 ` Darrick J. Wong
2017-01-05 1:04 ` [PATCH 3/7] ocfs2/reflink: fix file block size reporting Darrick J. Wong
2017-01-05 1:05 ` [PATCH 4/7] reflink: fix quota tests to work properly Darrick J. Wong
2017-01-09 8:55 ` Eryu Guan
2017-01-09 19:30 ` Darrick J. Wong
2017-01-09 20:53 ` [PATCH v2 " Darrick J. Wong
2017-01-05 1:05 ` [PATCH 5/7] reflink: make error reporting consistent when simulating EIO Darrick J. Wong
2017-01-05 1:05 ` [PATCH 6/7] dedupe: fix consistent error message prefixes for dedupe tests Darrick J. Wong
2017-01-09 9:26 ` Eryu Guan
2017-01-09 20:36 ` Darrick J. Wong
2017-01-09 20:54 ` [PATCH v2 " Darrick J. Wong
2017-01-05 1:05 ` [PATCH 7/7] xfs/ext4: check negative inode size Darrick J. Wong
2017-01-09 9:36 ` Eryu Guan
2017-01-09 20:36 ` Darrick J. Wong [this message]
2017-01-09 20:55 ` [PATCH v2 " Darrick J. Wong
2017-01-10 4:40 ` Eryu Guan
2017-01-10 4:52 ` Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2016-12-11 21:52 [PATCH 0/7] xfstests: misc reflink test fixes Darrick J. Wong
2016-12-11 21:53 ` [PATCH 7/7] xfs/ext4: check negative inode size Darrick J. Wong
2016-12-12 11:07 ` Eryu Guan
2016-12-13 21:49 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170109203626.GB14033@birch.djwong.org \
--to=darrick.wong@oracle.com \
--cc=eguan@redhat.com \
--cc=fstests@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.