* [PATCH 0/2] add tests for race between xattr read and write @ 2020-07-17 8:44 Hou Tao 2020-07-17 8:44 ` [PATCH 1/2] generic: add test for race between listxattr and setxatr Hou Tao 2020-07-17 8:44 ` [PATCH 2/2] generic: add test for race between getxattr and setxattr Hou Tao 0 siblings, 2 replies; 10+ messages in thread From: Hou Tao @ 2020-07-17 8:44 UTC (permalink / raw) To: guaneryu, Richard Weinberger; +Cc: fstests, houtao1 Hi, These two test cases are reproducers for xattr bugs [1] on ubifs. Although these bugs are ubifs specific, I still place these test cases under generic directory, because we can ensure other filesystems implement concurrent xattr read and write op correctly. Comments are welcome. Regards, Tao [1]: https://lore.kernel.org/linux-mtd/20200630130438.141649-1-houtao1@huawei.com/ Hou Tao (2): generic: add test for race between listxattr and setxatr generic: add test for race between getxattr and setxattr tests/generic/998 | 64 ++++++++++++++++++++++++++++++++++++++++ tests/generic/998.out | 2 ++ tests/generic/999 | 68 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/999.out | 2 ++ tests/generic/group | 2 ++ 5 files changed, 138 insertions(+) create mode 100644 tests/generic/998 create mode 100644 tests/generic/998.out create mode 100644 tests/generic/999 create mode 100644 tests/generic/999.out -- 2.25.0.4.g0ad7144999 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] generic: add test for race between listxattr and setxatr 2020-07-17 8:44 [PATCH 0/2] add tests for race between xattr read and write Hou Tao @ 2020-07-17 8:44 ` Hou Tao 2020-07-20 3:40 ` Chao Yu 2020-07-17 8:44 ` [PATCH 2/2] generic: add test for race between getxattr and setxattr Hou Tao 1 sibling, 1 reply; 10+ messages in thread From: Hou Tao @ 2020-07-17 8:44 UTC (permalink / raw) To: guaneryu, Richard Weinberger; +Cc: fstests, houtao1 Add reproducer for a bug on ubifs where listxattr() copies the newly created xattr names regardless of the remaining buffer size, fails the assertion of used buffer size, and may corrupt buffer memory. Signed-off-by: Hou Tao <houtao1@huawei.com> --- tests/generic/998 | 64 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/998.out | 2 ++ tests/generic/group | 1 + 3 files changed, 67 insertions(+) create mode 100644 tests/generic/998 create mode 100644 tests/generic/998.out diff --git a/tests/generic/998 b/tests/generic/998 new file mode 100644 index 00000000..b108a969 --- /dev/null +++ b/tests/generic/998 @@ -0,0 +1,64 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2020 Huawei. All Rights Reserved. +# +# FS QA Test 998 +# +# Test race between listxattr() and setxattr(). It reproduces a bug +# on UBIFS where listxattr() copies the newly created xattr names +# regardless of the remaining buffer size, fails the assertion of +# used buffer size, and may corrupt buffer memory. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $tmp.* + rm -f $TEST_DIR/$seq +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/attr + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here +_supported_fs generic +_supported_os Linux +_require_attrs + +target=$TEST_DIR/$seq +touch $target + +# start a background listxattr +runfile="$tmp.listxattr" +touch $runfile +while [ -e $runfile ]; do + ${GETFATTR_PROG} $target >/dev/null 2>&1 +done & + +# add new xattr continuously +largename=`for i in $(seq 0 128); do echo -n a; done` +for i in $(seq 0 99); do + ${SETFATTR_PROG} -n user.${largename}.$i -v $i $target +done + +rm -f $runfile +wait > /dev/null 2>&1 +rm -f $target + +echo Silence is golden + +# success, all done +status=0 +exit diff --git a/tests/generic/998.out b/tests/generic/998.out new file mode 100644 index 00000000..d2679ae0 --- /dev/null +++ b/tests/generic/998.out @@ -0,0 +1,2 @@ +QA output created by 998 +Silence is golden diff --git a/tests/generic/group b/tests/generic/group index d9ab9a31..62697ac5 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -605,3 +605,4 @@ 600 auto quick quota 601 auto quick quota 602 auto quick encrypt +998 auto quick attr -- 2.25.0.4.g0ad7144999 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] generic: add test for race between listxattr and setxatr 2020-07-17 8:44 ` [PATCH 1/2] generic: add test for race between listxattr and setxatr Hou Tao @ 2020-07-20 3:40 ` Chao Yu 2020-08-22 11:41 ` [PATCH v2 " Hou Tao 0 siblings, 1 reply; 10+ messages in thread From: Chao Yu @ 2020-07-20 3:40 UTC (permalink / raw) To: Hou Tao, guaneryu, Richard Weinberger; +Cc: fstests Hi Tao, On 2020/7/17 16:44, Hou Tao wrote: > Add reproducer for a bug on ubifs where listxattr() copies > the newly created xattr names regardless of the remaining > buffer size, fails the assertion of used buffer size, > and may corrupt buffer memory. > > Signed-off-by: Hou Tao <houtao1@huawei.com> > --- > tests/generic/998 | 64 +++++++++++++++++++++++++++++++++++++++++++ > tests/generic/998.out | 2 ++ > tests/generic/group | 1 + > 3 files changed, 67 insertions(+) > create mode 100644 tests/generic/998 > create mode 100644 tests/generic/998.out > > diff --git a/tests/generic/998 b/tests/generic/998 > new file mode 100644 > index 00000000..b108a969 > --- /dev/null > +++ b/tests/generic/998 > @@ -0,0 +1,64 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2020 Huawei. All Rights Reserved. > +# > +# FS QA Test 998 > +# > +# Test race between listxattr() and setxattr(). It reproduces a bug > +# on UBIFS where listxattr() copies the newly created xattr names > +# regardless of the remaining buffer size, fails the assertion of > +# used buffer size, and may corrupt buffer memory. > +# > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > + rm -f $TEST_DIR/$seq > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/attr > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# real QA test starts here > +_supported_fs generic > +_supported_os Linux > +_require_attrs > + > +target=$TEST_DIR/$seq > +touch $target > + > +# start a background listxattr > +runfile="$tmp.listxattr" > +touch $runfile > +while [ -e $runfile ]; do > + ${GETFATTR_PROG} $target >/dev/null 2>&1 > +done & > + > +# add new xattr continuously > +largename=`for i in $(seq 0 128); do echo -n a; done` > +for i in $(seq 0 99); do > + ${SETFATTR_PROG} -n user.${largename}.$i -v $i $target Now, f2fs just supports storing xattr in one 4KB size xattr block + inline space in inode, so this testcase will always fail with f2fs, would you please consider adjusting parameter to cover f2fs case? Thanks, > +done > + > +rm -f $runfile > +wait > /dev/null 2>&1 > +rm -f $target > + > +echo Silence is golden > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/998.out b/tests/generic/998.out > new file mode 100644 > index 00000000..d2679ae0 > --- /dev/null > +++ b/tests/generic/998.out > @@ -0,0 +1,2 @@ > +QA output created by 998 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index d9ab9a31..62697ac5 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -605,3 +605,4 @@ > 600 auto quick quota > 601 auto quick quota > 602 auto quick encrypt > +998 auto quick attr > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] generic: add test for race between listxattr and setxatr 2020-07-20 3:40 ` Chao Yu @ 2020-08-22 11:41 ` Hou Tao 2020-08-30 16:23 ` Eryu Guan 2020-09-02 16:43 ` Darrick J. Wong 0 siblings, 2 replies; 10+ messages in thread From: Hou Tao @ 2020-08-22 11:41 UTC (permalink / raw) To: guaneryu, richard, yuchao0; +Cc: fstests Add reproducer for a bug on ubifs where listxattr() copies the newly created xattr names regardless of the remaining buffer size, fails the assertion of used buffer size, and may corrupt buffer memory. Signed-off-by: Hou Tao <houtao1@huawei.com> --- v2: accommodate f2fs by reducing the number of created xattrs for f2fs tests/generic/998 | 67 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/998.out | 2 ++ tests/generic/group | 1 + 3 files changed, 70 insertions(+) create mode 100644 tests/generic/998 create mode 100644 tests/generic/998.out diff --git a/tests/generic/998 b/tests/generic/998 new file mode 100644 index 00000000..26a5b620 --- /dev/null +++ b/tests/generic/998 @@ -0,0 +1,67 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2020 Huawei. All Rights Reserved. +# +# FS QA Test 998 +# +# Test race between listxattr() and setxattr(). It reproduces a bug +# on UBIFS where listxattr() copies the newly created xattr names +# regardless of the remaining buffer size, fails the assertion of +# used buffer size, and may corrupt buffer memory. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $tmp.* + rm -f $TEST_DIR/$seq +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/attr + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here +_supported_fs generic +_supported_os Linux +_require_attrs + +target=$TEST_DIR/$seq +touch $target + +# start a background listxattr +runfile="$tmp.listxattr" +touch $runfile +while [ -e $runfile ]; do + ${GETFATTR_PROG} $target >/dev/null 2>&1 +done & + +# add new xattr continuously +largename=`for i in $(seq 0 128); do echo -n a; done` +cnt=100 +# f2fs has limited spaces for xattr +[ $FSTYP == "f2fs" ] && cnt=30 +for i in $(seq 1 $cnt); do + ${SETFATTR_PROG} -n user.${largename}.$i -v $i $target +done + +rm -f $runfile +wait > /dev/null 2>&1 +rm -f $target + +echo Silence is golden + +# success, all done +status=0 +exit diff --git a/tests/generic/998.out b/tests/generic/998.out new file mode 100644 index 00000000..d2679ae0 --- /dev/null +++ b/tests/generic/998.out @@ -0,0 +1,2 @@ +QA output created by 998 +Silence is golden diff --git a/tests/generic/group b/tests/generic/group index d9ab9a31..62697ac5 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -605,3 +605,4 @@ 600 auto quick quota 601 auto quick quota 602 auto quick encrypt +998 auto quick attr -- 2.25.0.4.g0ad7144999 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] generic: add test for race between listxattr and setxatr 2020-08-22 11:41 ` [PATCH v2 " Hou Tao @ 2020-08-30 16:23 ` Eryu Guan 2020-08-31 1:19 ` Chao Yu 2020-08-31 3:01 ` Hou Tao 2020-09-02 16:43 ` Darrick J. Wong 1 sibling, 2 replies; 10+ messages in thread From: Eryu Guan @ 2020-08-30 16:23 UTC (permalink / raw) To: Hou Tao; +Cc: guaneryu, richard, yuchao0, fstests On Sat, Aug 22, 2020 at 07:41:32PM +0800, Hou Tao wrote: > Add reproducer for a bug on ubifs where listxattr() copies > the newly created xattr names regardless of the remaining > buffer size, fails the assertion of used buffer size, > and may corrupt buffer memory. > > Signed-off-by: Hou Tao <houtao1@huawei.com> > --- > v2: accommodate f2fs by reducing the number of created xattrs for f2fs Thanks for the test and revision! Is there a fix available for the ubifs bug? If so would you please mention the kernel commit ID in commit log as well? Chao, would you please help check the update regarding to f2fs? Thanks! Eryu > > tests/generic/998 | 67 +++++++++++++++++++++++++++++++++++++++++++ > tests/generic/998.out | 2 ++ > tests/generic/group | 1 + > 3 files changed, 70 insertions(+) > create mode 100644 tests/generic/998 > create mode 100644 tests/generic/998.out > > diff --git a/tests/generic/998 b/tests/generic/998 > new file mode 100644 > index 00000000..26a5b620 > --- /dev/null > +++ b/tests/generic/998 > @@ -0,0 +1,67 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2020 Huawei. All Rights Reserved. > +# > +# FS QA Test 998 > +# > +# Test race between listxattr() and setxattr(). It reproduces a bug > +# on UBIFS where listxattr() copies the newly created xattr names > +# regardless of the remaining buffer size, fails the assertion of > +# used buffer size, and may corrupt buffer memory. > +# > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > + rm -f $TEST_DIR/$seq > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/attr > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# real QA test starts here > +_supported_fs generic > +_supported_os Linux > +_require_attrs > + > +target=$TEST_DIR/$seq > +touch $target > + > +# start a background listxattr > +runfile="$tmp.listxattr" > +touch $runfile > +while [ -e $runfile ]; do > + ${GETFATTR_PROG} $target >/dev/null 2>&1 > +done & > + > +# add new xattr continuously > +largename=`for i in $(seq 0 128); do echo -n a; done` > +cnt=100 > +# f2fs has limited spaces for xattr > +[ $FSTYP == "f2fs" ] && cnt=30 > +for i in $(seq 1 $cnt); do > + ${SETFATTR_PROG} -n user.${largename}.$i -v $i $target > +done > + > +rm -f $runfile > +wait > /dev/null 2>&1 > +rm -f $target > + > +echo Silence is golden > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/998.out b/tests/generic/998.out > new file mode 100644 > index 00000000..d2679ae0 > --- /dev/null > +++ b/tests/generic/998.out > @@ -0,0 +1,2 @@ > +QA output created by 998 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index d9ab9a31..62697ac5 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -605,3 +605,4 @@ > 600 auto quick quota > 601 auto quick quota > 602 auto quick encrypt > +998 auto quick attr > -- > 2.25.0.4.g0ad7144999 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] generic: add test for race between listxattr and setxatr 2020-08-30 16:23 ` Eryu Guan @ 2020-08-31 1:19 ` Chao Yu 2020-08-31 3:01 ` Hou Tao 1 sibling, 0 replies; 10+ messages in thread From: Chao Yu @ 2020-08-31 1:19 UTC (permalink / raw) To: Eryu Guan, Hou Tao; +Cc: guaneryu, richard, fstests On 2020/8/31 0:23, Eryu Guan wrote: > On Sat, Aug 22, 2020 at 07:41:32PM +0800, Hou Tao wrote: >> Add reproducer for a bug on ubifs where listxattr() copies >> the newly created xattr names regardless of the remaining >> buffer size, fails the assertion of used buffer size, >> and may corrupt buffer memory. >> >> Signed-off-by: Hou Tao <houtao1@huawei.com> >> --- >> v2: accommodate f2fs by reducing the number of created xattrs for f2fs > > Thanks for the test and revision! Is there a fix available for the ubifs > bug? If so would you please mention the kernel commit ID in commit log > as well? > > Chao, would you please help check the update regarding to f2fs? Thanks! Hi, all, Sorry for the delay. I've ran the test, it looks fine to f2fs, and also parameter value 'cnt=30' is just touch upper boundary of xattr space size. :) Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > > Eryu > >> >> tests/generic/998 | 67 +++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/998.out | 2 ++ >> tests/generic/group | 1 + >> 3 files changed, 70 insertions(+) >> create mode 100644 tests/generic/998 >> create mode 100644 tests/generic/998.out >> >> diff --git a/tests/generic/998 b/tests/generic/998 >> new file mode 100644 >> index 00000000..26a5b620 >> --- /dev/null >> +++ b/tests/generic/998 >> @@ -0,0 +1,67 @@ >> +#! /bin/bash >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (c) 2020 Huawei. All Rights Reserved. >> +# >> +# FS QA Test 998 >> +# >> +# Test race between listxattr() and setxattr(). It reproduces a bug >> +# on UBIFS where listxattr() copies the newly created xattr names >> +# regardless of the remaining buffer size, fails the assertion of >> +# used buffer size, and may corrupt buffer memory. >> +# >> +seq=`basename $0` >> +seqres=$RESULT_DIR/$seq >> +echo "QA output created by $seq" >> + >> +here=`pwd` >> +tmp=/tmp/$$ >> +status=1 # failure is the default! >> +trap "_cleanup; exit \$status" 0 1 2 3 15 >> + >> +_cleanup() >> +{ >> + cd / >> + rm -f $tmp.* >> + rm -f $TEST_DIR/$seq >> +} >> + >> +# get standard environment, filters and checks >> +. ./common/rc >> +. ./common/attr >> + >> +# remove previous $seqres.full before test >> +rm -f $seqres.full >> + >> +# real QA test starts here >> +_supported_fs generic >> +_supported_os Linux >> +_require_attrs >> + >> +target=$TEST_DIR/$seq >> +touch $target >> + >> +# start a background listxattr >> +runfile="$tmp.listxattr" >> +touch $runfile >> +while [ -e $runfile ]; do >> + ${GETFATTR_PROG} $target >/dev/null 2>&1 >> +done & >> + >> +# add new xattr continuously >> +largename=`for i in $(seq 0 128); do echo -n a; done` >> +cnt=100 >> +# f2fs has limited spaces for xattr >> +[ $FSTYP == "f2fs" ] && cnt=30 >> +for i in $(seq 1 $cnt); do >> + ${SETFATTR_PROG} -n user.${largename}.$i -v $i $target >> +done >> + >> +rm -f $runfile >> +wait > /dev/null 2>&1 >> +rm -f $target >> + >> +echo Silence is golden >> + >> +# success, all done >> +status=0 >> +exit >> diff --git a/tests/generic/998.out b/tests/generic/998.out >> new file mode 100644 >> index 00000000..d2679ae0 >> --- /dev/null >> +++ b/tests/generic/998.out >> @@ -0,0 +1,2 @@ >> +QA output created by 998 >> +Silence is golden >> diff --git a/tests/generic/group b/tests/generic/group >> index d9ab9a31..62697ac5 100644 >> --- a/tests/generic/group >> +++ b/tests/generic/group >> @@ -605,3 +605,4 @@ >> 600 auto quick quota >> 601 auto quick quota >> 602 auto quick encrypt >> +998 auto quick attr >> -- >> 2.25.0.4.g0ad7144999 > . > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] generic: add test for race between listxattr and setxatr 2020-08-30 16:23 ` Eryu Guan 2020-08-31 1:19 ` Chao Yu @ 2020-08-31 3:01 ` Hou Tao 2020-09-02 15:48 ` Eryu Guan 1 sibling, 1 reply; 10+ messages in thread From: Hou Tao @ 2020-08-31 3:01 UTC (permalink / raw) To: Eryu Guan, richard; +Cc: guaneryu, yuchao0, fstests Hi, On 2020/8/31 0:23, Eryu Guan wrote: > On Sat, Aug 22, 2020 at 07:41:32PM +0800, Hou Tao wrote: >> Add reproducer for a bug on ubifs where listxattr() copies >> the newly created xattr names regardless of the remaining >> buffer size, fails the assertion of used buffer size, >> and may corrupt buffer memory. >> >> Signed-off-by: Hou Tao <houtao1@huawei.com> >> --- >> v2: accommodate f2fs by reducing the number of created xattrs for f2fs > > Thanks for the test and revision! Is there a fix available for the ubifs > bug? If so would you please mention the kernel commit ID in commit log > as well? > The fixes [1] have not been reviewed yet, so we may need to hold on until the fixes get merged ? [1]: https://lore.kernel.org/linux-mtd/20200630130438.141649-1-houtao1@huawei.com/ Regards, Tao > Chao, would you please help check the update regarding to f2fs? Thanks! > > Eryu > >> >> tests/generic/998 | 67 +++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/998.out | 2 ++ >> tests/generic/group | 1 + >> 3 files changed, 70 insertions(+) >> create mode 100644 tests/generic/998 >> create mode 100644 tests/generic/998.out >> >> diff --git a/tests/generic/998 b/tests/generic/998 >> new file mode 100644 >> index 00000000..26a5b620 >> --- /dev/null >> +++ b/tests/generic/998 >> @@ -0,0 +1,67 @@ >> +#! /bin/bash >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (c) 2020 Huawei. All Rights Reserved. >> +# >> +# FS QA Test 998 >> +# >> +# Test race between listxattr() and setxattr(). It reproduces a bug >> +# on UBIFS where listxattr() copies the newly created xattr names >> +# regardless of the remaining buffer size, fails the assertion of >> +# used buffer size, and may corrupt buffer memory. >> +# >> +seq=`basename $0` >> +seqres=$RESULT_DIR/$seq >> +echo "QA output created by $seq" >> + >> +here=`pwd` >> +tmp=/tmp/$$ >> +status=1 # failure is the default! >> +trap "_cleanup; exit \$status" 0 1 2 3 15 >> + >> +_cleanup() >> +{ >> + cd / >> + rm -f $tmp.* >> + rm -f $TEST_DIR/$seq >> +} >> + >> +# get standard environment, filters and checks >> +. ./common/rc >> +. ./common/attr >> + >> +# remove previous $seqres.full before test >> +rm -f $seqres.full >> + >> +# real QA test starts here >> +_supported_fs generic >> +_supported_os Linux >> +_require_attrs >> + >> +target=$TEST_DIR/$seq >> +touch $target >> + >> +# start a background listxattr >> +runfile="$tmp.listxattr" >> +touch $runfile >> +while [ -e $runfile ]; do >> + ${GETFATTR_PROG} $target >/dev/null 2>&1 >> +done & >> + >> +# add new xattr continuously >> +largename=`for i in $(seq 0 128); do echo -n a; done` >> +cnt=100 >> +# f2fs has limited spaces for xattr >> +[ $FSTYP == "f2fs" ] && cnt=30 >> +for i in $(seq 1 $cnt); do >> + ${SETFATTR_PROG} -n user.${largename}.$i -v $i $target >> +done >> + >> +rm -f $runfile >> +wait > /dev/null 2>&1 >> +rm -f $target >> + >> +echo Silence is golden >> + >> +# success, all done >> +status=0 >> +exit >> diff --git a/tests/generic/998.out b/tests/generic/998.out >> new file mode 100644 >> index 00000000..d2679ae0 >> --- /dev/null >> +++ b/tests/generic/998.out >> @@ -0,0 +1,2 @@ >> +QA output created by 998 >> +Silence is golden >> diff --git a/tests/generic/group b/tests/generic/group >> index d9ab9a31..62697ac5 100644 >> --- a/tests/generic/group >> +++ b/tests/generic/group >> @@ -605,3 +605,4 @@ >> 600 auto quick quota >> 601 auto quick quota >> 602 auto quick encrypt >> +998 auto quick attr >> -- >> 2.25.0.4.g0ad7144999 > . > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] generic: add test for race between listxattr and setxatr 2020-08-31 3:01 ` Hou Tao @ 2020-09-02 15:48 ` Eryu Guan 0 siblings, 0 replies; 10+ messages in thread From: Eryu Guan @ 2020-09-02 15:48 UTC (permalink / raw) To: Hou Tao; +Cc: richard, guaneryu, yuchao0, fstests On Mon, Aug 31, 2020 at 11:01:05AM +0800, Hou Tao wrote: > Hi, > > On 2020/8/31 0:23, Eryu Guan wrote: > > On Sat, Aug 22, 2020 at 07:41:32PM +0800, Hou Tao wrote: > >> Add reproducer for a bug on ubifs where listxattr() copies > >> the newly created xattr names regardless of the remaining > >> buffer size, fails the assertion of used buffer size, > >> and may corrupt buffer memory. > >> > >> Signed-off-by: Hou Tao <houtao1@huawei.com> > >> --- > >> v2: accommodate f2fs by reducing the number of created xattrs for f2fs > > > > Thanks for the test and revision! Is there a fix available for the ubifs > > bug? If so would you please mention the kernel commit ID in commit log > > as well? > > > The fixes [1] have not been reviewed yet, so we may need to hold on until the fixes get merged ? > > [1]: https://lore.kernel.org/linux-mtd/20200630130438.141649-1-houtao1@huawei.com/ It's better to have the fixes merged first, but we do merge tests before the fixes landed in. But I'd like to see at least some reviews/acks on the fixes first. Thanks, Eryu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] generic: add test for race between listxattr and setxatr 2020-08-22 11:41 ` [PATCH v2 " Hou Tao 2020-08-30 16:23 ` Eryu Guan @ 2020-09-02 16:43 ` Darrick J. Wong 1 sibling, 0 replies; 10+ messages in thread From: Darrick J. Wong @ 2020-09-02 16:43 UTC (permalink / raw) To: Hou Tao; +Cc: guaneryu, richard, yuchao0, fstests On Sat, Aug 22, 2020 at 07:41:32PM +0800, Hou Tao wrote: > Add reproducer for a bug on ubifs where listxattr() copies > the newly created xattr names regardless of the remaining > buffer size, fails the assertion of used buffer size, > and may corrupt buffer memory. > > Signed-off-by: Hou Tao <houtao1@huawei.com> > --- > v2: accommodate f2fs by reducing the number of created xattrs for f2fs > > tests/generic/998 | 67 +++++++++++++++++++++++++++++++++++++++++++ > tests/generic/998.out | 2 ++ > tests/generic/group | 1 + > 3 files changed, 70 insertions(+) > create mode 100644 tests/generic/998 > create mode 100644 tests/generic/998.out > > diff --git a/tests/generic/998 b/tests/generic/998 > new file mode 100644 > index 00000000..26a5b620 > --- /dev/null > +++ b/tests/generic/998 > @@ -0,0 +1,67 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2020 Huawei. All Rights Reserved. > +# > +# FS QA Test 998 > +# > +# Test race between listxattr() and setxattr(). It reproduces a bug > +# on UBIFS where listxattr() copies the newly created xattr names > +# regardless of the remaining buffer size, fails the assertion of > +# used buffer size, and may corrupt buffer memory. > +# > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > + rm -f $TEST_DIR/$seq > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/attr > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# real QA test starts here > +_supported_fs generic > +_supported_os Linux > +_require_attrs > + > +target=$TEST_DIR/$seq > +touch $target > + > +# start a background listxattr > +runfile="$tmp.listxattr" > +touch $runfile > +while [ -e $runfile ]; do > + ${GETFATTR_PROG} $target >/dev/null 2>&1 Er, if the listxattr corrupts the buffer memory, how does this test detect that? Does the kernel crash or something? > +done & > + > +# add new xattr continuously > +largename=`for i in $(seq 0 128); do echo -n a; done` > +cnt=100 > +# f2fs has limited spaces for xattr > +[ $FSTYP == "f2fs" ] && cnt=30 > +for i in $(seq 1 $cnt); do > + ${SETFATTR_PROG} -n user.${largename}.$i -v $i $target Seeing as each filesystem has different xattr limits, I wonder if instead of special casing of f2fs we ought to capture the stdout/stderr of SETFATTR_PROG and filter out the ENOSPC message? AFAICT this test isn't trying to check that setting xattrs succeeds, but I'm a little confused on how we detect the corrupted buffer... --D > +done > + > +rm -f $runfile > +wait > /dev/null 2>&1 > +rm -f $target > + > +echo Silence is golden > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/998.out b/tests/generic/998.out > new file mode 100644 > index 00000000..d2679ae0 > --- /dev/null > +++ b/tests/generic/998.out > @@ -0,0 +1,2 @@ > +QA output created by 998 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index d9ab9a31..62697ac5 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -605,3 +605,4 @@ > 600 auto quick quota > 601 auto quick quota > 602 auto quick encrypt > +998 auto quick attr > -- > 2.25.0.4.g0ad7144999 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] generic: add test for race between getxattr and setxattr 2020-07-17 8:44 [PATCH 0/2] add tests for race between xattr read and write Hou Tao 2020-07-17 8:44 ` [PATCH 1/2] generic: add test for race between listxattr and setxatr Hou Tao @ 2020-07-17 8:44 ` Hou Tao 1 sibling, 0 replies; 10+ messages in thread From: Hou Tao @ 2020-07-17 8:44 UTC (permalink / raw) To: guaneryu, Richard Weinberger; +Cc: fstests, houtao1 Add reproducer for a bug on UBIFS where getxattr() may fail the assertions about the size of xattr value when the xattr is being modified or deleted simultaneously. Signed-off-by: Hou Tao <houtao1@huawei.com> --- tests/generic/999 | 68 +++++++++++++++++++++++++++++++++++++++++++ tests/generic/999.out | 2 ++ tests/generic/group | 1 + 3 files changed, 71 insertions(+) create mode 100644 tests/generic/999 create mode 100644 tests/generic/999.out diff --git a/tests/generic/999 b/tests/generic/999 new file mode 100644 index 00000000..5223d4aa --- /dev/null +++ b/tests/generic/999 @@ -0,0 +1,68 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2020 Huawei. All Rights Reserved. +# +# FS QA Test 999 +# +# Test race between getxattr() and setxattr(). It reproduces a bug +# on UBIFS where getxattr() may fail the assertions about the size of +# xattr value when the xattr is being modified or deleted simultaneously. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $tmp.* + rm -f $TEST_DIR/$seq +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/attr + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here +_supported_fs generic +_supported_os Linux +_require_attrs + +target=$TEST_DIR/$seq +touch $target + +xattr=user.test +${SETFATTR_PROG} -n $xattr -v 1 $target + +# start a background getxattr +runfile="$tmp.getxattr" +touch $runfile +while [ -e $runfile ]; do + ${GETFATTR_PROG} -n $xattr $target >/dev/null 2>&1 +done & + +# modify and remove xattr repeatedly +largeval=`for i in $(seq 0 128); do echo -n a; done` +for i in $(seq 0 99); do + ${SETFATTR_PROG} -n $xattr -v $largeval $target + ${SETFATTR_PROG} -x $xattr $target + ${SETFATTR_PROG} -n $xattr -v 1 $target +done + +rm -f $runfile +wait > /dev/null 2>&1 +rm -f $target + +echo Silence is golden + +# success, all done +status=0 +exit diff --git a/tests/generic/999.out b/tests/generic/999.out new file mode 100644 index 00000000..3b276ca8 --- /dev/null +++ b/tests/generic/999.out @@ -0,0 +1,2 @@ +QA output created by 999 +Silence is golden diff --git a/tests/generic/group b/tests/generic/group index 62697ac5..368399f1 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -606,3 +606,4 @@ 601 auto quick quota 602 auto quick encrypt 998 auto quick attr +999 auto quick attr -- 2.25.0.4.g0ad7144999 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-02 16:44 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-07-17 8:44 [PATCH 0/2] add tests for race between xattr read and write Hou Tao 2020-07-17 8:44 ` [PATCH 1/2] generic: add test for race between listxattr and setxatr Hou Tao 2020-07-20 3:40 ` Chao Yu 2020-08-22 11:41 ` [PATCH v2 " Hou Tao 2020-08-30 16:23 ` Eryu Guan 2020-08-31 1:19 ` Chao Yu 2020-08-31 3:01 ` Hou Tao 2020-09-02 15:48 ` Eryu Guan 2020-09-02 16:43 ` Darrick J. Wong 2020-07-17 8:44 ` [PATCH 2/2] generic: add test for race between getxattr and setxattr Hou Tao
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.