Re: [PATCH] ubifs: add test for race between listxattr and setxatr

[Eryu Guan <guan@eryu.me>]

On Sat, Jan 08, 2022 at 10:24:31AM +0800, Chen Long 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.
> 
> This is a regression test for three kernel commit:
> 1. f4e3634a3b642 (ubifs: Fix races between xattr_{set|get} and
> 	listxattr operations)
> 2. 819f9ab430a44 (ubifs: Remove ui_mutex in ubifs_xattr_get and
> 	change_xattr)

Only two commits listed above, but description mentioned 'three' kernel
commits. Missed one commit or should s/three/the/ ?

And I don't see any ubifs-specific setups or tests, I think it's
possible to make it a generic test.

> 
> Signed-off-by: Chen Long <chenlongcl.chen@huawei.com>
> ---
>  tests/ubifs/001      | 54 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/ubifs/001.out  |  2 ++
>  tests/ubifs/Makefile | 24 ++++++++++++++++++++
>  3 files changed, 80 insertions(+)
>  create mode 100755 tests/ubifs/001
>  create mode 100644 tests/ubifs/001.out
>  create mode 100644 tests/ubifs/Makefile
> 
> diff --git a/tests/ubifs/001 b/tests/ubifs/001
> new file mode 100755
> index 00000000..1e9a4de2
> --- /dev/null
> +++ b/tests/ubifs/001
> @@ -0,0 +1,54 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Huawei.  All Rights Reserved.
> +#
> +# FS QA Test 001
> +#
> +# Regression test for kernel commit:
> +# 1. f4e3634a3b642 (ubifs: Fix races between xattr_{set|get} and
> +#	listxattr operations)
> +# 2. 819f9ab430a44 (ubifs: Remove ui_mutex in ubifs_xattr_get and
> +#	change_xattr)
> +#
> +. ./common/preamble
> +_begin_fstest auto quick attr
> +
> +_cleanup()
> +{
> +    cd /
> +    rm -r -f $tmp.*
> +}

Use tab for indention, not spaces.

And this _cleanup() function is the same as the default one, so you
don't need to override it.

> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/attr
> +
> +# real QA test starts here
> +_supported_fs ubifs
> +_require_test
> +_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`

Don't have to loop for 129 times, perl could do it as

largename=`$PERL_PROG -e 'print "a"x129;'`

> +for i in $(seq 0 99); do

for i in {0..99}; do

which could avoid forking a new process :)

> +    ${SETFATTR_PROG} -n user.${largename}.$i -v $i $target
> +done

Thanks,
Eryu

> +
> +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/ubifs/001.out b/tests/ubifs/001.out
> new file mode 100644
> index 00000000..88678b8e
> --- /dev/null
> +++ b/tests/ubifs/001.out
> @@ -0,0 +1,2 @@
> +QA output created by 001
> +Silence is golden
> diff --git a/tests/ubifs/Makefile b/tests/ubifs/Makefile
> new file mode 100644
> index 00000000..b464b22b
> --- /dev/null
> +++ b/tests/ubifs/Makefile
> @@ -0,0 +1,24 @@
> +#
> +# Copyright (c) 2003-2005 Silicon Graphics, Inc.  All Rights Reserved.
> +#
> +
> +TOPDIR = ../..
> +include $(TOPDIR)/include/builddefs
> +include $(TOPDIR)/include/buildgrouplist
> +
> +GENERIC_DIR = generic
> +TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GENERIC_DIR)
> +DIRT = group.list
> +
> +default: $(DIRT)
> +
> +include $(BUILDRULES)
> +
> +install:
> +	$(INSTALL) -m 755 -d $(TARGET_DIR)
> +	$(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
> +	$(INSTALL) -m 644 group.list $(TARGET_DIR)
> +	$(INSTALL) -m 644 $(OUTFILES) $(TARGET_DIR)
> +
> +# Nothing.
> +install-dev install-lib:
> -- 
> 2.17.1