2022-07-26 03:14:56

by Sun Ke

Subject: [PATCH v4] ext4: resize fs after set reserved GDT blocks equals 100

Set reserved GDT blocks equals 100, then resize fs, will trigger null
pointer.

Regression test for commit b55c3cd102a6 ext4: add reserved GDT blocks
check.

Signed-off-by: Sun Ke <[email protected]>
---
tests/ext4/057 | 46 ++++++++++++++++++++++++++++++++++++++++++++++
tests/ext4/057.out | 2 ++
2 files changed, 48 insertions(+)
create mode 100755 tests/ext4/057
create mode 100644 tests/ext4/057.out

diff --git a/tests/ext4/057 b/tests/ext4/057
new file mode 100755
index 00000000..f4bbcd32
--- /dev/null
+++ b/tests/ext4/057
@@ -0,0 +1,46 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 HUAWEI. All Rights Reserved.
+#
+# FS QA Test 057
+#
+# Set reserved GDT blocks equals 100, then resize fs, will trigger null pointer.
+#
+# Regression test for commit
+# b55c3cd102a6 ext4: add reserved GDT blocks check
+#
+. ./common/preamble
+_begin_fstest auto resize quick
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs ext4
+_fixed_by_kernel_commit b55c3cd102a6 \
+ "ext4: add reserved GDT blocks check"
+
+_require_command "$TUNE2FS_PROG" tune2fs
+_require_command "$RESIZE2FS_PROG" resize2fs
+_require_command "$DEBUGFS_PROG" debugfs
+_require_scratch_size $((1024 * 1024)) #kB
+_require_scratch_nocheck
+
+# set fs size 512M
+dev_size=$((512 * 1024 * 1024))
+MKFS_OPTIONS="-O ^resize_inode $MKFS_OPTIONS" _scratch_mkfs_sized $dev_size \
+ >>$seqres.full 2>&1 || _fail "mkfs failed"
+
+$DEBUGFS_PROG -w -R "set_super_value s_reserved_gdt_blocks 100" $SCRATCH_DEV \
+ >>$seqres.full 2>&1
+
+$DEBUGFS_PROG -R "show_super_stats -h" $SCRATCH_DEV 2>/dev/null | \
+ grep "Reserved GDT blocks"
+
+_scratch_mount
+
+# resize fs will trigger NULL pointer in ext4_flex_group_add
+$RESIZE2FS_PROG $SCRATCH_DEV 1G >> $seqres.full 2>&1
+
+# success, all done
+status=0
+exit
diff --git a/tests/ext4/057.out b/tests/ext4/057.out
new file mode 100644
index 00000000..99423e16
--- /dev/null
+++ b/tests/ext4/057.out
@@ -0,0 +1,2 @@
+QA output created by 057
+Reserved GDT blocks: 100
--
2.13.6

2022-07-26 05:12:07

by Zorro Lang

Subject: Re: [PATCH v4] ext4: resize fs after set reserved GDT blocks equals 100

On Tue, Jul 26, 2022 at 11:22:40AM +0800, Sun Ke wrote:
> Set reserved GDT blocks equals 100, then resize fs, will trigger null
> pointer.
>
> Regression test for commit b55c3cd102a6 ext4: add reserved GDT blocks
> check.
>
> Signed-off-by: Sun Ke <[email protected]>
> ---
> tests/ext4/057 | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/ext4/057.out | 2 ++
> 2 files changed, 48 insertions(+)
> create mode 100755 tests/ext4/057
> create mode 100644 tests/ext4/057.out
>
> diff --git a/tests/ext4/057 b/tests/ext4/057
> new file mode 100755
> index 00000000..f4bbcd32
> --- /dev/null
> +++ b/tests/ext4/057
> @@ -0,0 +1,46 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 HUAWEI. All Rights Reserved.
> +#
> +# FS QA Test 057
> +#
> +# Set reserved GDT blocks equals 100, then resize fs, will trigger null pointer.

The number "100" isn't important, it just hope to get reserved GDT blocks
even if you disable resize_inode feature. So you'd better to reflect these
two conditions, rather than the number "100". E.g:

# A regression test for b55c3cd102a6 ("ext4: add reserved GDT blocks check").
# Make sure there's not kernel crash, if resize an ext4 which resize_inode
# feature is disabled but has reserved GDT blocks.

> +#
> +# Regression test for commit
> +# b55c3cd102a6 ext4: add reserved GDT blocks check
> +#
> +. ./common/preamble
> +_begin_fstest auto resize quick
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
^^^^
Remove this useless comment

> +_supported_fs ext4
> +_fixed_by_kernel_commit b55c3cd102a6 \
> + "ext4: add reserved GDT blocks check"
> +
> +_require_command "$TUNE2FS_PROG" tune2fs

If you don't need this command anymore, remove this line.

> +_require_command "$RESIZE2FS_PROG" resize2fs
> +_require_command "$DEBUGFS_PROG" debugfs
> +_require_scratch_size $((1024 * 1024)) #kB
> +_require_scratch_nocheck

Generally we don't use _require_scratch_size and _require_scratch together,
You can use a simple one line:

_require_scratch_size_nocheck $((1024 * 1024))

> +
> +# set fs size 512M

If you'd like to have some comments:

# Initalize a 512M ext4 fs with resize_inode feature disabled

> +dev_size=$((512 * 1024 * 1024))
> +MKFS_OPTIONS="-O ^resize_inode $MKFS_OPTIONS" _scratch_mkfs_sized $dev_size \
> + >>$seqres.full 2>&1 || _fail "mkfs failed"
> +

# Force some reserved GDT blocks to trigger the bug

> +$DEBUGFS_PROG -w -R "set_super_value s_reserved_gdt_blocks 100" $SCRATCH_DEV \
> + >>$seqres.full 2>&1
> +
> +$DEBUGFS_PROG -R "show_super_stats -h" $SCRATCH_DEV 2>/dev/null | \
> + grep "Reserved GDT blocks"
> +
> +_scratch_mount
> +
> +# resize fs will trigger NULL pointer in ext4_flex_group_add

# Expect no crash from this resize operation

> +$RESIZE2FS_PROG $SCRATCH_DEV 1G >> $seqres.full 2>&1
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/ext4/057.out b/tests/ext4/057.out
> new file mode 100644
> index 00000000..99423e16
> --- /dev/null
> +++ b/tests/ext4/057.out
> @@ -0,0 +1,2 @@
> +QA output created by 057
> +Reserved GDT blocks: 100
> --
> 2.13.6
>