lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 22 Jul 2022 16:16:26 +0800 From: Sun Ke <sunke32@...wei.com> To: Zorro Lang <zlang@...hat.com>, Theodore Ts'o <tytso@....edu> CC: <fstests@...r.kernel.org>, <linux-ext4@...r.kernel.org> Subject: Re: [PATCH v3 1/2] ext4: resize fs after resize_inode without e2fsck 在 2022/7/16 2:08, Zorro Lang 写道: > On Thu, Jul 14, 2022 at 06:00:34PM -0400, Theodore Ts'o wrote: >> On Thu, Jul 14, 2022 at 11:46:07PM +0800, Zorro Lang wrote: >>> On Wed, Jul 13, 2022 at 05:28:58PM +0800, Sun Ke wrote: >>>> + >>>> +# forget to run requested e2fsck after resize_inode >>>> +$TUNE2FS_PROG -O ^resize_inode $SCRATCH_DEV | grep -w "e2fsck" >>>> + >>>> +_scratch_mount >>>> + >>>> +# resize fs will trigger NULL pointer in ext4_flex_group_add >>>> +$RESIZE2FS_PROG $SCRATCH_DEV 1G >> $seqres.full 2>&1 >>>> + >>>> +echo "Silence is golden" >> ... >>>> diff --git a/tests/ext4/057.out b/tests/ext4/057.out >>>> new file mode 100644 >>>> index 00000000..4784ad7e >>>> --- /dev/null >>>> +++ b/tests/ext4/057.out >>>> @@ -0,0 +1,3 @@ >>>> +QA output created by 057 >>>> +Please run e2fsck -f on the filesystem. >>> >>> If you hope to match this line, means this case isn't "Silence is golden". >>> >>> I don't know why you'd to have this line, it looks not suit to be golden >>> image. If you'd like to make sure current ext4 supports "resize_inode" >>> feature, you can use: >>> _require_scratch_ext4_feature resize_inode >> >> That's not the problem. >> >> The "tune2fs -O ^resize_inode" command is printing that message as a >> reminder that it would be a Really Good idea to run e2fsck on the file >> system, because tune2fs doesn't completely remove the resize inode >> after turning off that feature. >> >> The commit which this test is trying to verify is that the kernel >> won't oops if the system adminsitrator ignores the rather explicit >> request: >> >> Please run e2fsck -f on the filesystem. >> >> ... and blithely mounts the file system without running fsck -f on the >> file system first. While it could be argued that a system >> administrator which fails to follow instructions deserves everything >> they get, we decided the as a quality of implementation issue, it >> would be better if the kernel didn't dereference a NULL pointer in >> that case. :-) >> >> The one thing I'll note is that it is possible that at some point in >> the future, tune2fs could be improved so that it cleanly removes the >> resize_inode when the resize inode feature is removed, so that running >> "fsck.ext4 -f" is no longer necessary. So if you want to future-proof > > Good to know :) > >> the test so it doesn't fail once tune2fs is made more idiot-proof, it >> might be better if the test did something like this: >> >> mke2fs -t ext4 -O ^resize_inode /dev/vdc 512m >> debugfs -w -R "set_super_value s_reserved_gdt_blocks 100" /dev/vdc > > So make sure there're reserved GDT blocks, even if disable resize_inode > feature. > >> mount -t ext4 /dev/vdc /vdc >> resize2fs /dev/vdc 1G > > Thanks Ted! That's really helpful to get review points from ext4 expert. > > Hi Ke, would you mind re-sending this case refer to above review points? > You can refer to below code, but I didn't test it, so please test and make > sure it works and can reproduce the bug. Feel free to improve it if something > wrong. > > _require_command "$DEBUGFS_PROG" debugfs > ... > > 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 > $RESIZE2FS_PROG $SCRATCH_DEV 1g >> $seqres.full 2>&1 > > > Thanks, > Zorro two questions. 1. The test run fsck automatically, and complain inconsistent,I think it need not run fsck. 2. It warn missing kernel fix, but the commit had merged. ##test log## [root@...tvm xfstests-dev]# ./check tests/ext4/057 FSTYP -- ext4 PLATFORM -- Linux/x86_64 testvm 5.10.0-00009-g08870cf6d88a #35 SMP Fri Jul 22 15:50:00 CST 2022 MKFS_OPTIONS -- /dev/sdb MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb /mnt/scratch ext4/057 5s ... _check_generic_filesystem: filesystem on /dev/sdb is inconsistent (see /root/xfstests-dev/results//ext4/057.full for details) HINT: You _MAY_ be missing kernel fix: b55c3cd102a6 ext4: add reserved GDT blocks check Ran: ext4/057 Failures: ext4/057 Failed 1 of 1 tests ##test## # Modify as appropriate. _supported_fs ext4 _fixed_by_kernel_commit b55c3cd102a6 \ "ext4: add reserved GDT blocks check" _require_scratch _require_command "$TUNE2FS_PROG" tune2fs _require_command "$RESIZE2FS_PROG" resize2fs _require_command "$DEBUGFS_PROG" debugfs _require_scratch_size $((1024 * 1024)) #kB # 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 > > >> >> Translating the above from commands suitable for manual trial using >> "kvm-xfstests shell" to a proper xfstests script is left as an >> exercise for the reader. :-) >> >> - Ted >> > > . >
Powered by blists - more mailing lists