| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 21 Feb 2022 11:11:32 +0530
From: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
To: Ritesh Harjani <riteshh@...ux.ibm.com>
Cc: fstests@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 2/2] ext4: Test to ensure resize with sparse_super2 is
handled correctly
Hi Ritesh,
Thanks for the review.
On Sat, Feb 19, 2022 at 12:52:11PM +0530, Ritesh Harjani wrote:
> On 22/02/07 01:55PM, Ojaswin Mujoo wrote:
> > Kernel currently doesn't support resize of EXT4 mounted
> > with sparse_super2 option enabled. Earlier, it used to leave the resize
> > incomplete and the fs would be left in an inconsistent state, however commit
> > b1489186cc83[1] fixed this to avoid the fs corruption by clearly returning
> > -ENOTSUPP.
> >
> > Test to ensure that kernel handles resizing with sparse_super2 correctly. Run
> > resize for multiple iterations because this leads to kernel crash due to
> > fs corruption, which we want to detect.
> >
> > Related commit in mainline:
> >
> > [1] commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61
> >
> > ext4: add check to prevent attempting to resize an fs with sparse_super2
>
> Thanks for the patch. Few nits below.
>
> >
> > Signed-off-by: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
> > ---
> > tests/ext4/056 | 102 +++++++++++++++++++++++++++++++++++++++++++++
> > tests/ext4/056.out | 2 +
> > 2 files changed, 104 insertions(+)
> > create mode 100755 tests/ext4/056
> > create mode 100644 tests/ext4/056.out
> >
> > diff --git a/tests/ext4/056 b/tests/ext4/056
> > new file mode 100755
> > index 00000000..9185621d
> > --- /dev/null
> > +++ b/tests/ext4/056
> > @@ -0,0 +1,102 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 IBM. All Rights Reserved.
> > +#
> > +# We don't currently support resize of EXT4 filesystems mounted
> > +# with sparse_super2 option enabled. Earlier, kernel used to leave the resize
> > +# incomplete and the fs would be left into an incomplete state, however commit
> > +# b1489186cc83 fixed this to avoid the fs corruption by clearly returning
> > +# -ENOTSUPP.
> > +#
> > +# This test ensures that kernel handles resizing with sparse_super2 correctly
> > +#
> > +# Related commit in mainline:
> > +#
> > +# commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61
> > +# ext4: add check to prevent attempting to resize an fs with sparse_super2
> > +#
> > +
> > +. ./common/preamble
> > +_begin_fstest auto ioctl resize quick
> > +
> > +# real QA test starts here
> > +
> > +INITIAL_FS_SIZE=1G
> > +RESIZED_FS_SIZE=$((2*1024*1024*1024)) # 2G
> > +ONLINE_RESIZE_BLOCK_LIMIT=$((256*1024*1024))
> > +
> > +_supported_fs ext4
> > +_require_scratch_size $(($RESIZED_FS_SIZE/1024))
> > +_require_test_program "ext4_resize"
> > +
> > +_log()
> > +{
> > + echo "$seq: $*" >> $seqres.full 2>&1
> > +}
> > +
> > +do_resize()
> > +{
> > +
> > + $MKFS_PROG `_scratch_mkfs_options -t ext4 -E resize=$ONLINE_RESIZE_BLOCK_LIMIT \
> > + -O sparse_super2` $INITIAL_FS_SIZE >> $seqres.full 2>&1 \
> > + || _fail "$MKFS_PROG failed. Exiting"
> > +
> > + _scratch_mount || _fail "Failed to mount scratch partition. Exiting"
> > +
> > + BS=$(_get_block_size $SCRATCH_MNT)
> > + NEW_BLOCKS=$(($RESIZED_FS_SIZE/$BS))
> > +
> > + local RESIZE_RET
> > + local EOPNOTSUPP=95
> > +
> > + $here/src/ext4_resize $SCRATCH_MNT $NEW_BLOCKS >> $seqres.full 2>&1
> > + RESIZE_RET=$?
> > +
> > + # Use $RESIZE_RET for logging
> > + if [ $RESIZE_RET = 0 ]
> > + then
> > + _log "Resizing succeeded but FS might still be corrupt."
>
> IMO, this above logs is not required. Putting iteration count is more than
> enough. You could log more info if the resize fails. But above log is somewhat
> confusing to me.
>
>
> > + elif [ $RESIZE_RET = $EOPNOTSUPP ]
> > + then
> > + _log "Resize operation not supported with sparse_super2"
> > + _log "Threw expected error $RESIZE_RET (EOPNOTSUPP)"
>
> If it fails with EOPNOTSUPP, do we still need to iterate in do_resize()
> 8 times?
>
> > +
> > + else
> > + _log "Output of resize = $RESIZE_RET. Expected $EOPNOTSUPP (EOPNOTSUPP)"
> > + _log "You might be missing kernel patch b1489186cc83"
>
> Not sure if we should pin point to a particular patch in this case.
> It could be that we add some features later and then some of those doesn't again
> support resize feature where it should return EOPNOTSUPP, but this test could
> capture that. So, I feel above may not be required.
>
> > + fi
> > +
> > + # unount after ioctl sometimes fails with "device busy" so add a small delay
> > + sleep 0.1
>
> Let's not add this sleep for EOPNOTSUPP case.
>
I've noted all the above points and I agree on them. I'll work on these
and send out a v2.
Thanks and regards,
Ojaswin
> > +
> > + _scratch_unmount >> $seqres.full 2>&1 || _fail "$UMOUNT_PROG failed. Exiting"
> > +}
> > +
> > +run_test()
> > +{
> > + local FSCK_RET
> > + local ITERS=8
> > +
> > + for i in $(seq 1 $ITERS)
> > + do
> > + _log "----------- Iteration: $i ------------"
> > + do_resize
> > + done
> > +
> > + _log "-------- Iterations Complete ---------"
> > + _log "Checking if FS is in consistent state"
> > + _check_scratch_fs
> > + FSCK_RET=$?
> > +
> > + return $FSCK_RET
> > +}
> > +
> > +run_test
> > +status=$?
> > +
> > +if [ "$status" -eq "0" ]
> > +then
> > + echo "Test Succeeded!" | tee -a $seqres.full
> > +fi
> > +
> > +exit
> > diff --git a/tests/ext4/056.out b/tests/ext4/056.out
> > new file mode 100644
> > index 00000000..41706284
> > --- /dev/null
> > +++ b/tests/ext4/056.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 056
> > +Test Succeeded!
> > --
> > 2.27.0
> >
Powered by blists - more mailing lists