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] [day] [month] [year] [list]
Message-ID: <9c6f6f09-d80a-450d-9a41-47de4d88469a@gmail.com>
Date: Wed, 15 Jan 2025 10:40:22 +0530
From: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>
To: "Ritesh Harjani (IBM)" <ritesh.list@...il.com>,
 Zorro Lang <zlang@...hat.com>
Cc: fstests@...r.kernel.org, linux-ext4@...r.kernel.org,
 linux-xfs@...r.kernel.org, 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 21:03, Ritesh Harjani (IBM) wrote:
> Zorro Lang <zlang@...hat.com> writes:
>
>> On Mon, Jan 13, 2025 at 02:22:20PM +0530, Nirjhar Roy (IBM) wrote:
>>> 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?
>> Hmm... the _init_rc doesn't do _test_mkfs,
> Yes, I had noticed that problem. So I felt sourcing fs specific file
> before _test_mkfs should be ok.
>
>> so you might need to do ". common/rc" after "_test_mkfs", rather than "_test_unmount".
>>
>> By checking the _init_rc, I think it can replace the _test_mount you metioned
>> above. Some details might need more testing, to make sure we didn't miss
>> anything wrong:)
> If moving . common/rc above _test_mount works, than that is a better
> approach than sourcing fs specific config file twice.

Yes, moving the ". common/rc" just after _test_mkfs and removing the 
_test_mount after fixes it the issue. I will do additional testing 
before sending a v2.

--NR

>
>
> -ritesh
>
>> Any review points from others?
>>
>> Thanks,
>> Zorro
>>
>>> --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
>>>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore