diff mbox series

ext4: verify ext4 when the starting block of index and leaf are inconsistent

Message ID 20211123013759.170012-1-chenlongcl.chen@huawei.com (mailing list archive)
State New, archived
Headers show
Series ext4: verify ext4 when the starting block of index and leaf are inconsistent | expand

Commit Message

Chen Long Nov. 23, 2021, 1:37 a.m. UTC 
From: chenlong <chenlongcl.chen@huawei.com>

Insert the extent entry in ext4_ext_insert_extent() (to be inserted at
the beginning of the block). In the stage of updating the starting block
number of the parent index block, an error happened
in ext4_ext_correct_indexes()->ext4_ext_get_access(), which caused the
index update of the parent index node to fail. The ext4_ext_insert_extent()
function exits directly and does not roll back the extent entry of
the leaf block. Eventually, the extent starting block numbers in
the index block and leaf block are inconsistent, triggering bugon

This is a regression test for three kernel commit:
1. 0f2f87d51aebc (ext4: prevent partial update of the extent blocks)
2. 9c6e071913792 (ext4: check for inconsistent extents between index
   and leaf block)
3. 8dd27fecede55 (ext4: check for out-of-order index extents in
   ext4_valid_extent_entries())
---
 tests/ext4/054     | 97 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/054.out |  2 +
 2 files changed, 99 insertions(+)
 create mode 100755 tests/ext4/054
 create mode 100644 tests/ext4/054.out

Comments

Eryu Guan Nov. 28, 2021, 4:15 p.m. UTC | #1 
On Tue, Nov 23, 2021 at 09:37:59AM +0800, chenlongcl.chen@huawei.com wrote:
> From: chenlong <chenlongcl.chen@huawei.com>
> 
> Insert the extent entry in ext4_ext_insert_extent() (to be inserted at
> the beginning of the block). In the stage of updating the starting block
> number of the parent index block, an error happened
> in ext4_ext_correct_indexes()->ext4_ext_get_access(), which caused the
> index update of the parent index node to fail. The ext4_ext_insert_extent()
> function exits directly and does not roll back the extent entry of
> the leaf block. Eventually, the extent starting block numbers in
> the index block and leaf block are inconsistent, triggering bugon
> 
> This is a regression test for three kernel commit:
> 1. 0f2f87d51aebc (ext4: prevent partial update of the extent blocks)
> 2. 9c6e071913792 (ext4: check for inconsistent extents between index
>    and leaf block)
> 3. 8dd27fecede55 (ext4: check for out-of-order index extents in
>    ext4_valid_extent_entries())

Missing Signed-off-by tag.

> ---
>  tests/ext4/054     | 97 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/ext4/054.out |  2 +
>  2 files changed, 99 insertions(+)
>  create mode 100755 tests/ext4/054
>  create mode 100644 tests/ext4/054.out
> 
> diff --git a/tests/ext4/054 b/tests/ext4/054
> new file mode 100755
> index 00000000..3d63f12f
> --- /dev/null
> +++ b/tests/ext4/054
> @@ -0,0 +1,97 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Huawei.  All Rights Reserved.
> +#
> +# FS QA Test 054
> +#
> +# Regression test for kernel commit:
> +# 1. 0f2f87d51aebc (ext4: prevent partial update of the extent blocks)
> +# 2. 9c6e071913792 (ext4: check for inconsistent extents between index \
> +#    and leaf block)
> +# 3. 8dd27fecede55 (ext4: check for out-of-order index extents in \
> +#    ext4_valid_extent_entries())
> +
> +. ./common/preamble
> +_begin_fstest auto quick
> +
> +_cleanup()
> +{
> +    cd /
> +    rm -f $tmp.*
> +    $UMOUNT_PROG ${IMG_MNT} > /dev/null 2>&1
> +    rm -f ${IMG_FILE} > /dev/null 2>&1
> +}
> +
> +# Import common functions
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs ext4
> +_require_test
> +_require_xfs_io_command "falloc"
> +_require_xfs_io_command "pwrite"
> +_require_xfs_io_command "fsync"
> +_require_xfs_io_command "fpunch"
> +_require_command "$DEBUGFS_PROG" debugfs
> +
> +IMG_FILE=${SCRATCH_MNT}/$seq.fs
> +IMG_MNT=${SCRATCH_MNT}/$seq.mnt

SCRATCH_DEV is not required nor mounted, so IMG_FILE and IMG_MNT are
created on the root filesystem.

And I don't see why we need to loop back mount an fs image here, does
_require_scratch_nocheck then use $SCRATCH_DEV directly work for you?

And if loop device is really needed, _require_loop should be added as
well, and use $TEST_DIR to save the fs image and mnt dir, e.g.

IMG_FILE=$TEST_DIR/$seq.fs
IMG_MNT=$TEST_DIR/$seq.mnt

and use _create_loop_device and _destroy_loop_device helpers.

> +TEST_FILE="${IMG_MNT}/testfile"
> +
> +fallocate -l 10M $IMG_FILE

$XFS_IO_PROG -c "falloc 0 10m" $IMG_FILE

> +mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
> +
> +$MKFS_EXT4_PROG -F $IMG_FILE >> $seqres.full 2>&1 || _fail "mkfs failed"
> +
> +$MOUNT_PROG -t ${FSTYP} ${IMG_FILE} ${IMG_MNT} >/dev/null 2>&1 || \
> +    _fail "mount failed"

If use $SCRATCH_DEV, above will simply be

_scratch_mkfs >> $seqres.full 2>&1
_scratch_mount 

> +touch $TEST_FILE
> +
> +#
> +# The following steps create an abnormal file system image and
> +# create a file(testfile). The starting logic block of his second leaf
                                                          ^^^^ its ?

> +# extent block is inconsistent with the starting logic block of the
> +# second idx extent entry in the inode.
> +# Level Entries       Logical      Physical Length Flags
> +# 0/ 1   2/  2  5632 -  6527  2020            896
> +#               ^^^^ 指向了leaf block
                        ^^^^^ Use English please.

> +# 1/ 1   1/ 15  5376 -  5439  7041 -  7104     64 Uninit
> +# 1/ 1   2/ 15  5632 -  5695  7297 -  7360     64 Uninit
> +#
> +for i in {0..50}
> +do
> +    offset=$((1024 * 128 * i))
> +    $XFS_IO_PROG -c "falloc $offset $((1024 * 64))" $TEST_FILE >> $seqres.full
> +    offset=$((offset + 1024 * 64))
> +    $XFS_IO_PROG -c "pwrite $offset $((1024 * 64))" $TEST_FILE >> $seqres.full
> +    $XFS_IO_PROG -c "fsync" $TEST_FILE >> $seqres.full
> +done
> +
> +$XFS_IO_PROG -c "fpunch $((1024 * 5376)) $((1024 * 256))" $TEST_FILE \
> +        >> $seqres.full
> +$XFS_IO_PROG -c "fsync" $TEST_FILE >> $seqres.full
> +$XFS_IO_PROG -c "falloc $((1024 * 5376)) $((1024 * 64))" $TEST_FILE \
> +        >> $seqres.full
> +$XFS_IO_PROG -c "fsync" $TEST_FILE
> +
> +$UMOUNT_PROG ${IMG_MNT} || _fail "umount failed"
> +
> +$DEBUGFS_PROG -w -R "set_inode_field testfile block[6] 0x1600" $IMG_FILE \
> +        2>>$seqres.full >> $seqres.full
> +
> +# Mount the file system and create a block at a location in the middle
> +# of 5440-5632
> +
> +$MOUNT_PROG -t ${FSTYP} -o nodelalloc ${IMG_FILE} ${IMG_MNT} \

Please add comments to explain why nodelalloc is needed.

Thanks,
Eryu

> +        >/dev/null 2>&1 || _fail "mount failed"
> +$XFS_IO_PROG -c "pwrite $((1024 * 5568)) $((1024 * 64))" $TEST_FILE \
> +        2>>$seqres.full >> $seqres.full
> +
> +# If this BUG is not fixed, when the testcase is executed to this position,
> +# the kernel will be BUG_ON immediately.
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/ext4/054.out b/tests/ext4/054.out
> new file mode 100644
> index 00000000..108fe4c9
> --- /dev/null
> +++ b/tests/ext4/054.out
> @@ -0,0 +1,2 @@
> +QA output created by 054
> +Silence is golden
> -- 
> 2.17.1
Chen Long Nov. 29, 2021, 3:41 a.m. UTC | #2 
On 2021/11/29 0:15, Eryu Guan wrote:
> On Tue, Nov 23, 2021 at 09:37:59AM +0800, chenlongcl.chen@huawei.com wrote:
>> From: chenlong <chenlongcl.chen@huawei.com>
>>
>> Insert the extent entry in ext4_ext_insert_extent() (to be inserted at
>> the beginning of the block). In the stage of updating the starting block
>> number of the parent index block, an error happened
>> in ext4_ext_correct_indexes()->ext4_ext_get_access(), which caused the
>> index update of the parent index node to fail. The ext4_ext_insert_extent()
>> function exits directly and does not roll back the extent entry of
>> the leaf block. Eventually, the extent starting block numbers in
>> the index block and leaf block are inconsistent, triggering bugon
>>
>> This is a regression test for three kernel commit:
>> 1. 0f2f87d51aebc (ext4: prevent partial update of the extent blocks)
>> 2. 9c6e071913792 (ext4: check for inconsistent extents between index
>>     and leaf block)
>> 3. 8dd27fecede55 (ext4: check for out-of-order index extents in
>>     ext4_valid_extent_entries())
> 
> Missing Signed-off-by tag.
>

Thanks for your review.
I will add it in the V2 version.

>> ---
>>   tests/ext4/054     | 97 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/ext4/054.out |  2 +
>>   2 files changed, 99 insertions(+)
>>   create mode 100755 tests/ext4/054
>>   create mode 100644 tests/ext4/054.out
>>
>> diff --git a/tests/ext4/054 b/tests/ext4/054
>> new file mode 100755
>> index 00000000..3d63f12f
>> --- /dev/null
>> +++ b/tests/ext4/054
>> @@ -0,0 +1,97 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2021 Huawei.  All Rights Reserved.
>> +#
>> +# FS QA Test 054
>> +#
>> +# Regression test for kernel commit:
>> +# 1. 0f2f87d51aebc (ext4: prevent partial update of the extent blocks)
>> +# 2. 9c6e071913792 (ext4: check for inconsistent extents between index \
>> +#    and leaf block)
>> +# 3. 8dd27fecede55 (ext4: check for out-of-order index extents in \
>> +#    ext4_valid_extent_entries())
>> +
>> +. ./common/preamble
>> +_begin_fstest auto quick
>> +
>> +_cleanup()
>> +{
>> +    cd /
>> +    rm -f $tmp.*
>> +    $UMOUNT_PROG ${IMG_MNT} > /dev/null 2>&1
>> +    rm -f ${IMG_FILE} > /dev/null 2>&1
>> +}
>> +
>> +# Import common functions
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +_supported_fs ext4
>> +_require_test
>> +_require_xfs_io_command "falloc"
>> +_require_xfs_io_command "pwrite"
>> +_require_xfs_io_command "fsync"
>> +_require_xfs_io_command "fpunch"
>> +_require_command "$DEBUGFS_PROG" debugfs
>> +
>> +IMG_FILE=${SCRATCH_MNT}/$seq.fs
>> +IMG_MNT=${SCRATCH_MNT}/$seq.mnt
> 
> SCRATCH_DEV is not required nor mounted, so IMG_FILE and IMG_MNT are
> created on the root filesystem.
> 
> And I don't see why we need to loop back mount an fs image here, does
> _require_scratch_nocheck then use $SCRATCH_DEV directly work for you?
> 
> And if loop device is really needed, _require_loop should be added as
> well, and use $TEST_DIR to save the fs image and mnt dir, e.g.
> 
> IMG_FILE=$TEST_DIR/$seq.fs
> IMG_MNT=$TEST_DIR/$seq.mnt
> 
> and use _create_loop_device and _destroy_loop_device helpers.
> 

The original idea was to construct a damaged file system by
creating a loop device. In fact, scratch_dev can also achieve
the goal. In V2 version, it will be changed to scratch_dev.

>> +TEST_FILE="${IMG_MNT}/testfile"
>> +
>> +fallocate -l 10M $IMG_FILE
> 
> $XFS_IO_PROG -c "falloc 0 10m" $IMG_FILE
>

use scratch_dev to test, no need to create img_file.

>> +mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
>> +
>> +$MKFS_EXT4_PROG -F $IMG_FILE >> $seqres.full 2>&1 || _fail "mkfs failed"
>> +
>> +$MOUNT_PROG -t ${FSTYP} ${IMG_FILE} ${IMG_MNT} >/dev/null 2>&1 || \
>> +    _fail "mount failed"
> 
> If use $SCRATCH_DEV, above will simply be
> 
> _scratch_mkfs >> $seqres.full 2>&1
> _scratch_mount
>

no need to use loop device.

>> +touch $TEST_FILE
>> +
>> +#
>> +# The following steps create an abnormal file system image and
>> +# create a file(testfile). The starting logic block of his second leaf
>                                                            ^^^^ its ?
>

it is more appropriate to use its here.

>> +# extent block is inconsistent with the starting logic block of the
>> +# second idx extent entry in the inode.
>> +# Level Entries       Logical      Physical Length Flags
>> +# 0/ 1   2/  2  5632 -  6527  2020            896
>> +#               ^^^^ 指向了leaf block
>                          ^^^^^ Use English please.
>

sorry, i correct it.

>> +# 1/ 1   1/ 15  5376 -  5439  7041 -  7104     64 Uninit
>> +# 1/ 1   2/ 15  5632 -  5695  7297 -  7360     64 Uninit
>> +#
>> +for i in {0..50}
>> +do
>> +    offset=$((1024 * 128 * i))
>> +    $XFS_IO_PROG -c "falloc $offset $((1024 * 64))" $TEST_FILE >> $seqres.full
>> +    offset=$((offset + 1024 * 64))
>> +    $XFS_IO_PROG -c "pwrite $offset $((1024 * 64))" $TEST_FILE >> $seqres.full
>> +    $XFS_IO_PROG -c "fsync" $TEST_FILE >> $seqres.full
>> +done
>> +
>> +$XFS_IO_PROG -c "fpunch $((1024 * 5376)) $((1024 * 256))" $TEST_FILE \
>> +        >> $seqres.full
>> +$XFS_IO_PROG -c "fsync" $TEST_FILE >> $seqres.full
>> +$XFS_IO_PROG -c "falloc $((1024 * 5376)) $((1024 * 64))" $TEST_FILE \
>> +        >> $seqres.full
>> +$XFS_IO_PROG -c "fsync" $TEST_FILE
>> +
>> +$UMOUNT_PROG ${IMG_MNT} || _fail "umount failed"
>> +
>> +$DEBUGFS_PROG -w -R "set_inode_field testfile block[6] 0x1600" $IMG_FILE \
>> +        2>>$seqres.full >> $seqres.full
>> +
>> +# Mount the file system and create a block at a location in the middle
>> +# of 5440-5632
>> +
>> +$MOUNT_PROG -t ${FSTYP} -o nodelalloc ${IMG_FILE} ${IMG_MNT} \
> 
> Please add comments to explain why nodelalloc is needed.
> 
> Thanks,
> Eryu
>

When the delalloc option is enabled when mounting, the block allocation
will not be triggered when writing (the maximum waiting time is about
30s). If nodellalloc is used, the block will be allocated when writing.
The expected BUG_ON in this use case can be triggered immediately, and
the test results can be quickly obtained.

I will add comments in the V2 version.

>> +        >/dev/null 2>&1 || _fail "mount failed"
>> +$XFS_IO_PROG -c "pwrite $((1024 * 5568)) $((1024 * 64))" $TEST_FILE \
>> +        2>>$seqres.full >> $seqres.full
>> +
>> +# If this BUG is not fixed, when the testcase is executed to this position,
>> +# the kernel will be BUG_ON immediately.
>> +
>> +echo "Silence is golden"
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/ext4/054.out b/tests/ext4/054.out
>> new file mode 100644
>> index 00000000..108fe4c9
>> --- /dev/null
>> +++ b/tests/ext4/054.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 054
>> +Silence is golden
>> -- 
>> 2.17.1
> .
>
diff mbox series

Patch

diff --git a/tests/ext4/054 b/tests/ext4/054
new file mode 100755
index 00000000..3d63f12f
--- /dev/null
+++ b/tests/ext4/054
@@ -0,0 +1,97 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Huawei.  All Rights Reserved.
+#
+# FS QA Test 054
+#
+# Regression test for kernel commit:
+# 1. 0f2f87d51aebc (ext4: prevent partial update of the extent blocks)
+# 2. 9c6e071913792 (ext4: check for inconsistent extents between index \
+#    and leaf block)
+# 3. 8dd27fecede55 (ext4: check for out-of-order index extents in \
+#    ext4_valid_extent_entries())
+
+. ./common/preamble
+_begin_fstest auto quick
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+    $UMOUNT_PROG ${IMG_MNT} > /dev/null 2>&1
+    rm -f ${IMG_FILE} > /dev/null 2>&1
+}
+
+# Import common functions
+. ./common/filter
+
+# real QA test starts here
+_supported_fs ext4
+_require_test
+_require_xfs_io_command "falloc"
+_require_xfs_io_command "pwrite"
+_require_xfs_io_command "fsync"
+_require_xfs_io_command "fpunch"
+_require_command "$DEBUGFS_PROG" debugfs
+
+IMG_FILE=${SCRATCH_MNT}/$seq.fs
+IMG_MNT=${SCRATCH_MNT}/$seq.mnt
+TEST_FILE="${IMG_MNT}/testfile"
+
+fallocate -l 10M $IMG_FILE
+mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
+
+$MKFS_EXT4_PROG -F $IMG_FILE >> $seqres.full 2>&1 || _fail "mkfs failed"
+
+$MOUNT_PROG -t ${FSTYP} ${IMG_FILE} ${IMG_MNT} >/dev/null 2>&1 || \
+    _fail "mount failed"
+touch $TEST_FILE
+
+#
+# The following steps create an abnormal file system image and
+# create a file(testfile). The starting logic block of his second leaf
+# extent block is inconsistent with the starting logic block of the
+# second idx extent entry in the inode.
+# Level Entries       Logical      Physical Length Flags
+# 0/ 1   2/  2  5632 -  6527  2020            896
+#               ^^^^ 指向了leaf block
+# 1/ 1   1/ 15  5376 -  5439  7041 -  7104     64 Uninit
+# 1/ 1   2/ 15  5632 -  5695  7297 -  7360     64 Uninit
+#
+for i in {0..50}
+do
+    offset=$((1024 * 128 * i))
+    $XFS_IO_PROG -c "falloc $offset $((1024 * 64))" $TEST_FILE >> $seqres.full
+    offset=$((offset + 1024 * 64))
+    $XFS_IO_PROG -c "pwrite $offset $((1024 * 64))" $TEST_FILE >> $seqres.full
+    $XFS_IO_PROG -c "fsync" $TEST_FILE >> $seqres.full
+done
+
+$XFS_IO_PROG -c "fpunch $((1024 * 5376)) $((1024 * 256))" $TEST_FILE \
+        >> $seqres.full
+$XFS_IO_PROG -c "fsync" $TEST_FILE >> $seqres.full
+$XFS_IO_PROG -c "falloc $((1024 * 5376)) $((1024 * 64))" $TEST_FILE \
+        >> $seqres.full
+$XFS_IO_PROG -c "fsync" $TEST_FILE
+
+$UMOUNT_PROG ${IMG_MNT} || _fail "umount failed"
+
+$DEBUGFS_PROG -w -R "set_inode_field testfile block[6] 0x1600" $IMG_FILE \
+        2>>$seqres.full >> $seqres.full
+
+# Mount the file system and create a block at a location in the middle
+# of 5440-5632
+
+$MOUNT_PROG -t ${FSTYP} -o nodelalloc ${IMG_FILE} ${IMG_MNT} \
+        >/dev/null 2>&1 || _fail "mount failed"
+$XFS_IO_PROG -c "pwrite $((1024 * 5568)) $((1024 * 64))" $TEST_FILE \
+        2>>$seqres.full >> $seqres.full
+
+# If this BUG is not fixed, when the testcase is executed to this position,
+# the kernel will be BUG_ON immediately.
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/ext4/054.out b/tests/ext4/054.out
new file mode 100644
index 00000000..108fe4c9
--- /dev/null
+++ b/tests/ext4/054.out
@@ -0,0 +1,2 @@ 
+QA output created by 054
+Silence is golden