* [PATCH v2] xfs/126: Add a getxattr opeartion after corrupted xattr @ 2021-11-25 5:13 Yang Xu 2021-11-26 23:28 ` Darrick J. Wong 0 siblings, 1 reply; 8+ messages in thread From: Yang Xu @ 2021-11-25 5:13 UTC (permalink / raw) To: fstests; +Cc: Yang Xu It is design to reproduce a deadlock on upstream kernel. It is introduced by kernel commit 07120f1abdff ("xfs: Add xfs_has_attr and subroutines") and fixed by kernel commit a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname")[1]. [1]https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=a1de97fe296c Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- tests/xfs/126 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/xfs/126 b/tests/xfs/126 index c3a74b1c..5496f3d7 100755 --- a/tests/xfs/126 +++ b/tests/xfs/126 @@ -7,6 +7,10 @@ # Create and populate an XFS filesystem, corrupt a leaf xattr's data extent, # then see how the kernel and xfs_repair deal with it. # +# It is also a regression test for kernel commit: +# a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname") +# + . ./common/preamble _begin_fstest fuzzers @@ -69,7 +73,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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] xfs/126: Add a getxattr opeartion after corrupted xattr 2021-11-25 5:13 [PATCH v2] xfs/126: Add a getxattr opeartion after corrupted xattr Yang Xu @ 2021-11-26 23:28 ` Darrick J. Wong 2022-01-06 1:41 ` xuyang2018.jy 2022-01-17 8:22 ` [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted Yang Xu 0 siblings, 2 replies; 8+ messages in thread From: Darrick J. Wong @ 2021-11-26 23:28 UTC (permalink / raw) To: Yang Xu; +Cc: fstests On Thu, Nov 25, 2021 at 01:13:27PM +0800, Yang Xu wrote: > It is design to reproduce a deadlock on upstream kernel. It is introduced by kernel > commit 07120f1abdff ("xfs: Add xfs_has_attr and subroutines") and fixed by kernel > commit a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname")[1]. > > [1]https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=a1de97fe296c NAK, this is a general fuzz test. Please create a separate targeted regression test for the kernel bugfix so that you can control exactly which part of the attr leaf block gets corrupted so that the verifier will signal error. --D > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > tests/xfs/126 | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/tests/xfs/126 b/tests/xfs/126 > index c3a74b1c..5496f3d7 100755 > --- a/tests/xfs/126 > +++ b/tests/xfs/126 > @@ -7,6 +7,10 @@ > # Create and populate an XFS filesystem, corrupt a leaf xattr's data extent, > # then see how the kernel and xfs_repair deal with it. > # > +# It is also a regression test for kernel commit: > +# a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname") > +# > + > . ./common/preamble > _begin_fstest fuzzers > > @@ -69,7 +73,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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] xfs/126: Add a getxattr opeartion after corrupted xattr 2021-11-26 23:28 ` Darrick J. Wong @ 2022-01-06 1:41 ` xuyang2018.jy 2022-01-17 8:22 ` [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted Yang Xu 1 sibling, 0 replies; 8+ messages in thread From: xuyang2018.jy @ 2022-01-06 1:41 UTC (permalink / raw) To: Darrick J. Wong; +Cc: fstests on 2021/11/27 7:28, Darrick J. Wong wrote: > On Thu, Nov 25, 2021 at 01:13:27PM +0800, Yang Xu wrote: >> It is design to reproduce a deadlock on upstream kernel. It is introduced by kernel >> commit 07120f1abdff ("xfs: Add xfs_has_attr and subroutines") and fixed by kernel >> commit a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname")[1]. >> >> [1]https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=a1de97fe296c > > NAK, this is a general fuzz test. Please create a separate targeted > regression test for the kernel bugfix so that you can control exactly > which part of the attr leaf block gets corrupted so that the verifier > will signal error. Sorry for the late reply. IMO, getattr or setattr should all report metadata corrupt in dmesg(command fail)after the attr leaf block gets corrupted. I think it should be a common part, not only for this case also for other attr corrupt case ie xfs/125. We should add getattr for them after attr corrupt to avoid similar problem occurs other places. what do you think about this? ps: I should update my commit message for this. Best Regards Yang Xu > > --D > >> >> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com> >> --- >> tests/xfs/126 | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/tests/xfs/126 b/tests/xfs/126 >> index c3a74b1c..5496f3d7 100755 >> --- a/tests/xfs/126 >> +++ b/tests/xfs/126 >> @@ -7,6 +7,10 @@ >> # Create and populate an XFS filesystem, corrupt a leaf xattr's data extent, >> # then see how the kernel and xfs_repair deal with it. >> # >> +# It is also a regression test for kernel commit: >> +# a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname") >> +# >> + >> . ./common/preamble >> _begin_fstest fuzzers >> >> @@ -69,7 +73,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 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted 2021-11-26 23:28 ` Darrick J. Wong 2022-01-06 1:41 ` xuyang2018.jy @ 2022-01-17 8:22 ` Yang Xu 2022-01-23 13:09 ` Eryu Guan 1 sibling, 1 reply; 8+ messages in thread From: Yang Xu @ 2022-01-17 8:22 UTC (permalink / raw) To: djwong; +Cc: fstests, Yang Xu 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 commit a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname"). 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted 2022-01-17 8:22 ` [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted Yang Xu @ 2022-01-23 13:09 ` Eryu Guan 2022-01-27 3:02 ` xuyang2018.jy 0 siblings, 1 reply; 8+ messages in thread From: Eryu Guan @ 2022-01-23 13:09 UTC (permalink / raw) To: Yang Xu; +Cc: djwong, fstests 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. 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted 2022-01-23 13:09 ` Eryu Guan @ 2022-01-27 3:02 ` xuyang2018.jy 2022-01-27 4:59 ` Zorro Lang 0 siblings, 1 reply; 8+ messages in thread From: xuyang2018.jy @ 2022-01-27 3:02 UTC (permalink / raw) To: Eryu Guan; +Cc: djwong, fstests 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted 2022-01-27 3:02 ` xuyang2018.jy @ 2022-01-27 4:59 ` Zorro Lang 2022-01-30 2:44 ` xuyang2018.jy 0 siblings, 1 reply; 8+ messages in thread From: Zorro Lang @ 2022-01-27 4:59 UTC (permalink / raw) To: xuyang2018.jy; +Cc: Eryu Guan, djwong, fstests On Thu, Jan 27, 2022 at 03:02:36AM +0000, xuyang2018.jy@fujitsu.com wrote: > 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. Generally, if you keep creating attr entries, until the attr out of an inode internal attrfork space, you'll get leaf block and separated attr value blocks. If you keep creating more attr entries, until the attr entries out of a block size, you'll get node and leaf blocks. From xfs/126, Darrick use a while loop to create attrs in about 8 blocks. nr="$((8 * blksz / 40))" seq 0 "${nr}" | while read d; do setfattr -n "user.x$(printf "%.08d" "$d")" -v "0000000000000000" "${SCRATCH_MNT}/attrfile" done (Although I don't know why it's 40. From above setfattr command, I think each xfs_attr_leaf_name_local takes 28 bytes, each xfs_attr_leaf_entry takes 8 bytes. And there're node and leaf blocks, not sure how Darrick knows it'll take 8 blocks :). I didn't give it a test, not sure if Darrick would like to get 1 node block and 7 leaf blocks at here. Then he loop trash each ablocks (from 1 to end), except the root node block. loff=1 while true; do _scratch_xfs_db -x -c "inode ${inode}" -c "ablock ${loff}" -c "stack" | grep -q 'file attr block is unmapped' && break _scratch_xfs_db -x -c "inode ${inode}" -c "ablock ${loff}" -c "stack" -c "blocktrash -o 4 -x 32 -y $((blksz * 8)) -z ${FUZZ_ARGS}" >> $seqres.full loff="$((loff + 1))" done Thanks, Zorro > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted 2022-01-27 4:59 ` Zorro Lang @ 2022-01-30 2:44 ` xuyang2018.jy 0 siblings, 0 replies; 8+ messages in thread From: xuyang2018.jy @ 2022-01-30 2:44 UTC (permalink / raw) To: Zorro Lang; +Cc: Eryu Guan, djwong, fstests on 2022/1/27 12:59, Zorro Lang wrote: > On Thu, Jan 27, 2022 at 03:02:36AM +0000, xuyang2018.jy@fujitsu.com wrote: >> 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. > > Generally, if you keep creating attr entries, until the attr out of an inode > internal attrfork space, you'll get leaf block and separated attr value blocks. > If you keep creating more attr entries, until the attr entries out of a block > size, you'll get node and leaf blocks. > > From xfs/126, Darrick use a while loop to create attrs in about 8 blocks. > > nr="$((8 * blksz / 40))" > seq 0 "${nr}" | while read d; do > setfattr -n "user.x$(printf "%.08d" "$d")" -v "0000000000000000" "${SCRATCH_MNT}/attrfile" > done > > (Although I don't know why it's 40. From above setfattr command, I think each xfs_attr_leaf_name_local > takes 28 bytes, each xfs_attr_leaf_entry takes 8 bytes. And there're node and leaf blocks, not sure > how Darrick knows it'll take 8 blocks :). > > I didn't give it a test, not sure if Darrick would like to get 1 node block and 7 leaf blocks at here. > Then he loop trash each ablocks (from 1 to end), except the root node block. > > loff=1 > while true; do > _scratch_xfs_db -x -c "inode ${inode}" -c "ablock ${loff}" -c "stack" | grep -q 'file attr block is unmapped'&& break > _scratch_xfs_db -x -c "inode ${inode}" -c "ablock ${loff}" -c "stack" -c "blocktrash -o 4 -x 32 -y $((blksz * 8)) -z ${FUZZ_ARGS}">> $seqres.full > loff="$((loff + 1))" > done > > Thanks, > Zorro Thanks for your explaination, I will figure out each detail after Chinese Lunar New Year. Happy Chinese Lunar New Year in advance. Best Regards Yang Xu > > > >> >> 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-30 2:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-25 5:13 [PATCH v2] xfs/126: Add a getxattr opeartion after corrupted xattr Yang Xu 2021-11-26 23:28 ` Darrick J. Wong 2022-01-06 1:41 ` xuyang2018.jy 2022-01-17 8:22 ` [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted Yang Xu 2022-01-23 13:09 ` Eryu Guan 2022-01-27 3:02 ` xuyang2018.jy 2022-01-27 4:59 ` Zorro Lang 2022-01-30 2:44 ` xuyang2018.jy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.