linux-ext4 - Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD

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]

Message-ID: <4afe80a4-ac6b-4796-b466-c42a95f7225a@gmail.com>
Date: Mon, 13 Jan 2025 14:22:20 +0530
From: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>
To: Zorro Lang <zlang@...hat.com>
Cc: fstests@...r.kernel.org, linux-ext4@...r.kernel.org,
 linux-xfs@...r.kernel.org, ritesh.list@...il.com, ojaswin@...ux.ibm.com,
 djwong@...nel.org, zlang@...nel.org
Subject: Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE


On 1/13/25 11:29, Zorro Lang wrote:
> On Sun, Jan 12, 2025 at 03:21:51PM +0000, Nirjhar Roy (IBM) wrote:
>> Bug Description:
>>
>> _test_mount function is failing with the following error:
>> ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
>> check: failed to mount /dev/loop0 on /mnt1/test
>>
>> when the second section in local.config file is xfs and the first section
>> is non-xfs.
>>
>> It can be easily reproduced with the following local.config file
>>
>> [s2]
>> export FSTYP=ext4
>> export TEST_DEV=/dev/loop0
>> export TEST_DIR=/mnt1/test
>> export SCRATCH_DEV=/dev/loop1
>> export SCRATCH_MNT=/mnt1/scratch
>>
>> [s1]
>> export FSTYP=xfs
>> export TEST_DEV=/dev/loop0
>> export TEST_DIR=/mnt1/test
>> export SCRATCH_DEV=/dev/loop1
>> export SCRATCH_MNT=/mnt1/scratch
>>
>> ./check selftest/001
>>
>> Root cause:
>> When _test_mount() is executed for the second section, the FSTYPE has
>> already changed but the new fs specific common/$FSTYP has not yet
>> been done. Hence _xfs_prepare_for_eio_shutdown() is not found and
>> the test run fails.
>>
>> Fix:
>> call _source_specific_fs $FSTYP at the correct call site of  _test_mount()
>>
>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@...il.com>
>> ---
>>   check | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/check b/check
>> index 607d2456..8cdbb68f 100755
>> --- a/check
>> +++ b/check
>> @@ -776,6 +776,7 @@ function run_section()
>>   	if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
>>   		echo "RECREATING    -- $FSTYP on $TEST_DEV"
>>   		_test_unmount 2> /dev/null
>> +		[[ "$OLD_FSTYP" != "$FSTYP" ]] && _source_specific_fs $FSTYP
> The _source_specific_fs is called when importing common/rc file:
>
>    # check for correct setup and source the $FSTYP specific functions now
>    _source_specific_fs $FSTYP
>
>  From the code logic of check script:
>
>          if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
>                  echo "RECREATING    -- $FSTYP on $TEST_DEV"
>                  _test_unmount 2> /dev/null
>                  if ! _test_mkfs >$tmp.err 2>&1
>                  then
>                          echo "our local _test_mkfs routine ..."
>                          cat $tmp.err
>                          echo "check: failed to mkfs \$TEST_DEV using specified options"
>                          status=1
>                          exit
>                  fi
>                  if ! _test_mount
>                  then
>                          echo "check: failed to mount $TEST_DEV on $TEST_DIR"
>                          status=1
>                          exit
>                  fi
>                  # TEST_DEV has been recreated, previous FSTYP derived from
>                  # TEST_DEV could be changed, source common/rc again with
>                  # correct FSTYP to get FSTYP specific configs, e.g. common/xfs
>                  . common/rc
>                  ^^^^^^^^^^^
> we import common/rc at here.
>
> So I'm wondering if we can move this line upward, to fix the problem you
> hit (and don't bring in regression :) Does that help?
>
> Thanks,
> Zorro

Okay so we can move ". common/rc" upward and then remove the following 
from "check" file:

         if ! _test_mount
         then
             echo "check: failed to mount $TEST_DEV on $TEST_DIR"
             status=1
             exit
         fi

since . common/rc will call init_rc() in the end, which does a 
_test_mount(). Do you agree with this (Zorro and Ritesh)?

I can make the changes and send a v2?

--NR

>
>
>>   		if ! _test_mkfs >$tmp.err 2>&1
>>   		then
>>   			echo "our local _test_mkfs routine ..."
>> -- 
>> 2.34.1
>>
>>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore