linux-kernel - Re: [PATCH xfstests] overlay: Enable character device to be the base fs partition

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Sep 2019 22:19:38 +0800
From:   Zhihao Cheng <chengzhihao1@...wei.com>
To:     Amir Goldstein <amir73il@...il.com>
CC:     Eryu Guan <guaneryu@...il.com>,
        David Oberhollenzer <david.oberhollenzer@...ma-star.at>,
        Eric Biggers <ebiggers@...gle.com>,
        "zhangyi (F)" <yi.zhang@...wei.com>,
        fstests <fstests@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH xfstests] overlay: Enable character device to be the base
 fs partition

As far as I know, _require_scratch_shutdown() is called after _overay_config_override(), at this moment, FSTYP equals to base fs. According the implementation of _require_scratch_shutdown:
3090 _require_scratch_shutdown()
3091 {
3092     [ -x src/godown ] || _notrun "src/godown executable not found"
3093
3094     _scratch_mkfs > /dev/null 2>&1 || _notrun "_scratch_mkfs failed on $SCRATCH_DEV"
3095     _scratch_mount
3096
3097     if [ $FSTYP = "overlay" ]; then                                            # FSTYP = base fs
3098         if [ -z $OVL_BASE_SCRATCH_DEV ]; then
3099             # In lagacy overlay usage, it may specify directory as
3100             # SCRATCH_DEV, in this case OVL_BASE_SCRATCH_DEV
3101             # will be null, so check OVL_BASE_SCRATCH_DEV before
3102             # running shutdown to avoid shutting down base fs accidently.
3103             _notrun "$SCRATCH_DEV is not a block device"
3104         else
3105             src/godown -f $OVL_BASE_SCRATCH_MNT 2>&1 \
3106             || _notrun "Underlying filesystem does not support shutdown"
3107         fi
3108     else
3109         src/godown -f $SCRATCH_MNT 2>&1 \
3110             || _notrun "$FSTYP does not support shutdown"                      # Executes this path
3111     fi
3112
3113     _scratch_unmount
3114 }
So, we can't get output: _notrun "$SCRATCH_DEV is not a block device". Instead, the verbose should like:
  after _overlay_config_override FSTYP=ubifs    # Additional print message
  FSTYP         -- ubifs
  PLATFORM      -- Linux/x86_64
  MKFS_OPTIONS  -- /dev/ubi0_1
  MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch

  generic/042	[not run] ubifs does not support shutdown

But I'll consider describing error more concisely in v2.

在 2019/9/24 20:33, Amir Goldstein 写道:
> On Tue, Sep 24, 2019 at 12:34 PM Zhihao Cheng <chengzhihao1@...wei.com> wrote:
>>
>> When running overlay tests using character devices as base fs partitions,
>> all overlay usecase results become 'notrun'. Function
>> '_overay_config_override' (common/config) detects that the current base
>> fs partition is not a block device and will set FSTYP to base fs. The
>> overlay usecase will check the current FSTYP, and if it is not 'overlay'
>> or 'generic', it will skip the execution.
>>
>> For example, using UBIFS as base fs skips all overlay usecases:
>>
>>   FSTYP         -- ubifs       # FSTYP should be overridden as 'overlay'
>>   MKFS_OPTIONS  -- /dev/ubi0_1 # Character device
>>   MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch
>>
>>   overlay/001   [not run] not suitable for this filesystem type: ubifs
>>   overlay/002   [not run] not suitable for this filesystem type: ubifs
>>   overlay/003   [not run] not suitable for this filesystem type: ubifs
>>   ...
>>
>> When checking that the base fs partition is a block/character device,
>> FSTYP is overwritten as 'overlay'. This patch allows the base fs
>> partition to be a character device that can also execute overlay
>> usecases (such as ubifs).
>>
>> Signed-off-by: Zhihao Cheng <chengzhihao1@...wei.com>
>> ---
>>  common/config | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/config b/common/config
>> index 4c86a49..a22acdb 100644
>> --- a/common/config
>> +++ b/common/config
>> @@ -550,7 +550,7 @@ _overlay_config_override()
>>         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
>>         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
>>         #    overlayfs base and mount dirs inside base fs mount.
>> -       [ -b "$TEST_DEV" ] || return 0
>> +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
>>
>>         # Config file may specify base fs type, but we obay -overlay flag
>>         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
>> @@ -570,7 +570,7 @@ _overlay_config_override()
>>         export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
>>         export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
>>
>> -       [ -b "$SCRATCH_DEV" ] || return 0
>> +       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
>>
>>         # Store original base fs vars
>>         export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
>> --
>> 2.7.4
>>
> 
> Looks fine.
> 
> One nit: there is a message in _require_scratch_shutdown():
> _notrun "$SCRATCH_DEV is not a block device"
> for when $OVL_BASE_SCRATCH_DEV is not defined.
> 
> Could probably use a better describing error anyway.
> 
> Thanks,
> Amir.
> 
> .
>