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: <4f489dbf-b47f-4b1a-9299-e6d0b6ef2369@gmail.com>
Date: Wed, 15 Jan 2025 10:36:22 +0530
From: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>
To: "Ritesh Harjani (IBM)" <ritesh.list@...il.com>, fstests@...r.kernel.org
Cc: 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:09, Ritesh Harjani (IBM) wrote:
> "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com> writes:
>
>> On 1/13/25 01:11, Ritesh Harjani (IBM) wrote:
>>> "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com> writes:
>>>
>>>> Bug Description:
>>>>
>>>> _test_mount function is failing with the following error:
>>>> ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
> Please notice the error that you are seeing here ^^^
>
>>>> 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()
>>> You should add the Fixes: tag too. Based on your description I guess
>>> this should be the tag?
>>>
>>> Fixes: 1a49022fab9b4 ("fstests: always use fail-at-unmount semantics for XFS")
> Please look into the above commit. The above patch introduced function
> "_prepare_for_eio_shutdown()" in _test_mount(), which is what we are
> getting the error for (for XFS i.e. _xfs_prepare_for_eio_shutdown()
> command not found). Right?
Okay, now I got the logic. I was thinking of the commit that added the 
entire "if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ];" block 
(where the sourcing should have been there)
>
> Ok, why don't revert the above commit and see if the revert fixes the
> issue for you.
Yes, I can check that.
>
> https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

Thanks, I will read this.

--NR

>
> -ritesh
>
>> Shouldn't this be the following?
>>
>> commit f8e4f532f18d7517430d9849bfc042305d7f7f4d (HEAD)
>> Author: Lukas Czerner <lczerner@...hat.com>
>> Date:   Fri Apr 4 17:18:15 2014 +1100
>>
>>       check: Allow to recreate TEST_DEV
>>
>>       Add config option RECREATE_TEST_DEV to allow to recreate file system on
>>       the TEST_DEV device. Permitted values are true and false.
>>
>>       If RECREATE_TEST_DEV is set to true the TEST_DEV device will be
>>       unmounted and FSTYP file system will be created on it. Afterwards it
>>       will be mounted to TEST_DIR again with the default, or specified mount
>>       options.
>>
>>       Also recreate the file system if FSTYP differs from the previous
>>       section.
>>
>>> I agree with today the problem was in _test_mount(), tomorrow it could
>>> be _test_mkfs, hence we could source the new FSTYP config file before
>>> calling _test_mkfs().
>>>
>>> With the fixes tag added, please feel free to add:
>>>
>>> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@...il.com>
>>>
>>>> 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
>>>>    		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