Re: [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

on 2022/1/23 21:09, Eryu Guan wrote:
> On Mon, Jan 17, 2022 at 04:22:16PM +0800, Yang Xu wrote:
>> Add getfattr operation in these three cases, we can also use xfs/126(corrupt a leaf
>> xattr's data extent) to reproduce a deadlock. This deadlock is introduced by kernel
>> commit 07120f1abdff ("xfs: Add xfs_has_attr and subroutines") and fixed by kernel
>
> So this was introduced since 5.9 kernel, and
>
>> commit a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname").
>
> fixed in 5.16 kernel.
>
> And adding this regression test in xfs/126, which passed in the past.
> probably would cause all the affected kernels in between start to hang,
> and trigger a false positive regression alert.
>
> So IMO it'd be better to make a separated&  targeted regression test for
> this case, as Darrick suggested.
Ok, Now, I understand but don't figure out how to setattr  to use leaf 
attr block accuratly and corrupt leaf attr accuratly even I have read 
the documentation[1] instead of xfs/126 using generic fuzz way.

So if somebody can do this or teach me, I will be pleasure.

[1]https://github.com/djwong/xfs-documentation/blob/master/design/XFS_Filesystem_Structure/extended_attributes.asciidoc


Best Regards
Yang Xu
>
> Thanks,
> Eryu
>
>>
>> Also making getfattr operation after xattr corrupted is a common part, so we can
>> test whether the similar deadlock occurs in the future in xfs/124(corrupt a block xattr)
>> and xfs/125(corrupt a leaf xattr's index extent).
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>>   tests/xfs/124 | 2 +-
>>   tests/xfs/125 | 2 +-
>>   tests/xfs/126 | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/xfs/124 b/tests/xfs/124
>> index 39307218..c3cccfd5 100755
>> --- a/tests/xfs/124
>> +++ b/tests/xfs/124
>> @@ -64,7 +64,7 @@ _scratch_xfs_db -x -c "inode ${inode}" -c 'ablock 0' -c "stack" -c "blocktrash -
>>
>>   echo "+ mount image&&  modify xattr"
>>   if _try_scratch_mount>>  $seqres.full 2>&1; then
>> -
>> +	getfattr "${SCRATCH_MNT}/attrfile" -n "user.x00000000" 2>  /dev/null&&  _fail "got corrupt xattr"
>>   	setfattr -x "user.x00000000" "${SCRATCH_MNT}/attrfile" 2>  /dev/null&&  _fail "modified corrupt xattr"
>>   	umount "${SCRATCH_MNT}"
>>   fi
>> diff --git a/tests/xfs/125 b/tests/xfs/125
>> index fb5f5695..a7eb51fb 100755
>> --- a/tests/xfs/125
>> +++ b/tests/xfs/125
>> @@ -64,7 +64,7 @@ _scratch_xfs_db -x -c "inode ${inode}" -c 'ablock 0' -c "stack" -c "blocktrash -
>>
>>   echo "+ mount image&&  modify xattr"
>>   if _try_scratch_mount>>  $seqres.full 2>&1; then
>> -
>> +	getfattr "${SCRATCH_MNT}/attrfile" -n "user.x00000000" 2>  /dev/null&&  _fail "got corrupt xattr"
>>   	setfattr -x "user.x00000000" "${SCRATCH_MNT}/attrfile" 2>  /dev/null&&  _fail "modified corrupt xattr"
>>   	umount "${SCRATCH_MNT}"
>>   fi
>> diff --git a/tests/xfs/126 b/tests/xfs/126
>> index c3a74b1c..519d377e 100755
>> --- a/tests/xfs/126
>> +++ b/tests/xfs/126
>> @@ -69,7 +69,7 @@ done
>>
>>   echo "+ mount image&&  modify xattr"
>>   if _try_scratch_mount>>  $seqres.full 2>&1; then
>> -
>> +	getfattr "${SCRATCH_MNT}/attrfile" -n "user.x00000000" 2>  /dev/null&&  _fail "got corrupt xattr"
>>   	setfattr -x "user.x00000000" "${SCRATCH_MNT}/attrfile" 2>  /dev/null&&  _fail "modified corrupt xattr"
>>   	umount "${SCRATCH_MNT}"
>>   fi
>> --
>> 2.23.0