diff mbox series

sysctl/sysctl03: Test whether return EINVAL when passing negative value

Message ID 1643349071-2164-1-git-send-email-xuyang2018.jy@fujitsu.com
State Rejected
Headers show
Series sysctl/sysctl03: Test whether return EINVAL when passing negative value | expand

Commit Message

Yang Xu Jan. 28, 2022, 5:51 a.m. UTC
It is a regression for kernel commit
1622ed7d07432 ("sysctl: returns -EINVAL when a negative value is passed to proc_doulongvec_minmax").

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/commands                      |  1 +
 testcases/commands/sysctl/sysctl03.sh | 52 +++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100755 testcases/commands/sysctl/sysctl03.sh

Comments

Cyril Hrubis Jan. 28, 2022, 10:31 a.m. UTC | #1
Hi!
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
> +# Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> +#
> +# When we pass a negative value to the proc_doulongvec_minmax() function,
> +# the function should return EINVAL and the corresponding interface value
> +# does not change.
> +#
> +# It is also a regression test for
> +# commit 1622ed7d0743 ("sysctl: returns -EINVAL when a negative value is passed
> +# to proc_doulongvec_minmax").
> +#
> +# Use fs.epoll.max_user_watches interface to test this.
> +
> +TST_TESTFUNC=do_test
> +TST_SETUP=setup
> +TST_NEEDS_ROOT=1
> +TST_NEEDS_CMDS="sysctl"
> +TST_NEEDS_KCONFIGS="CONFIG_SYSCTL=y, CONFIG_PROC_FS=y, CONFIG_INOTIFY_USER=y"
> +sys_name="fs.epoll.max_user_watches"
> +sys_file="/proc/sys/fs/epoll/max_user_watches"
> +
> +. tst_test.sh
> +
> +setup()
> +{
> +	orig_value=$(cat "$sys_file")
> +}
> +
> +do_test()
> +{
> +	tst_res TINFO "trying to set $sys_name=-1"
> +
> +	sysctl -w -q $sys_name=-1 2>&1 | grep -q "Invalid argument"
> +
> +	if [ $? -eq 0 ]; then
> +		tst_res TPASS "sysctl returns EINVAL when passing a negative value"
> +	else
> +		tst_res TFAIL "sysctl doesn't return EINVAL when passing a negative value"
> +	fi
> +
> +	new_value=$(cat "$sys_file")
> +	if [ $orig_value -eq $new_value ]; then
> +		tst_res TPASS "the interface value doesn't change when passing a negative value"
> +	else
> +		tst_res TFAIL "the interface value changes when passing a negative value"
> +		sysctl -w -q $sys_name=$orig_value
> +	fi
> +}
> +
> +tst_run

I'm just wondering if this wouldn't be actually easier as a C test. I
bet that simple write() to the sys_file would work exactly the same and
we wouldn't depend on sysctl being present on the system.
Yang Xu Jan. 30, 2022, 2:24 a.m. UTC | #2
Hi Cyril
> Hi!
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +# Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
>> +# Author: Yang Xu<xuyang2018.jy@fujitsu.com>
>> +#
>> +# When we pass a negative value to the proc_doulongvec_minmax() function,
>> +# the function should return EINVAL and the corresponding interface value
>> +# does not change.
>> +#
>> +# It is also a regression test for
>> +# commit 1622ed7d0743 ("sysctl: returns -EINVAL when a negative value is passed
>> +# to proc_doulongvec_minmax").
>> +#
>> +# Use fs.epoll.max_user_watches interface to test this.
>> +
>> +TST_TESTFUNC=do_test
>> +TST_SETUP=setup
>> +TST_NEEDS_ROOT=1
>> +TST_NEEDS_CMDS="sysctl"
>> +TST_NEEDS_KCONFIGS="CONFIG_SYSCTL=y, CONFIG_PROC_FS=y, CONFIG_INOTIFY_USER=y"
>> +sys_name="fs.epoll.max_user_watches"
>> +sys_file="/proc/sys/fs/epoll/max_user_watches"
>> +
>> +. tst_test.sh
>> +
>> +setup()
>> +{
>> +	orig_value=$(cat "$sys_file")
>> +}
>> +
>> +do_test()
>> +{
>> +	tst_res TINFO "trying to set $sys_name=-1"
>> +
>> +	sysctl -w -q $sys_name=-1 2>&1 | grep -q "Invalid argument"
>> +
>> +	if [ $? -eq 0 ]; then
>> +		tst_res TPASS "sysctl returns EINVAL when passing a negative value"
>> +	else
>> +		tst_res TFAIL "sysctl doesn't return EINVAL when passing a negative value"
>> +	fi
>> +
>> +	new_value=$(cat "$sys_file")
>> +	if [ $orig_value -eq $new_value ]; then
>> +		tst_res TPASS "the interface value doesn't change when passing a negative value"
>> +	else
>> +		tst_res TFAIL "the interface value changes when passing a negative value"
>> +		sysctl -w -q $sys_name=$orig_value
>> +	fi
>> +}
>> +
>> +tst_run
>
> I'm just wondering if this wouldn't be actually easier as a C test. I
> bet that simple write() to the sys_file would work exactly the same and
> we wouldn't depend on sysctl being present on the system.
If using C program, it seems not have a suitable place to put this test 
case. Do you have some advise?

Best Regards
Yang Xu
>
Cyril Hrubis Feb. 16, 2022, 12:59 p.m. UTC | #3
Hi!
> > I'm just wondering if this wouldn't be actually easier as a C test. I
> > bet that simple write() to the sys_file would work exactly the same and
> > we wouldn't depend on sysctl being present on the system.
> If using C program, it seems not have a suitable place to put this test 
> case. Do you have some advise?

We do have testcases/kernel/fs/proc/ so I guess that we can as well add
testcases/kernel/fs/sys/
diff mbox series

Patch

diff --git a/runtest/commands b/runtest/commands
index 8cfad0449..c515c7231 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -41,4 +41,5 @@  gdb01_sh gdb01.sh
 unshare01_sh unshare01.sh
 sysctl01_sh sysctl01.sh
 sysctl02_sh sysctl02.sh
+sysctl03_sh sysctl03.sh
 shell_test01 echo "SUCCESS" | shell_pipe01.sh
diff --git a/testcases/commands/sysctl/sysctl03.sh b/testcases/commands/sysctl/sysctl03.sh
new file mode 100755
index 000000000..ae7bd11c6
--- /dev/null
+++ b/testcases/commands/sysctl/sysctl03.sh
@@ -0,0 +1,52 @@ 
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+# Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+#
+# When we pass a negative value to the proc_doulongvec_minmax() function,
+# the function should return EINVAL and the corresponding interface value
+# does not change.
+#
+# It is also a regression test for
+# commit 1622ed7d0743 ("sysctl: returns -EINVAL when a negative value is passed
+# to proc_doulongvec_minmax").
+#
+# Use fs.epoll.max_user_watches interface to test this.
+
+TST_TESTFUNC=do_test
+TST_SETUP=setup
+TST_NEEDS_ROOT=1
+TST_NEEDS_CMDS="sysctl"
+TST_NEEDS_KCONFIGS="CONFIG_SYSCTL=y, CONFIG_PROC_FS=y, CONFIG_INOTIFY_USER=y"
+sys_name="fs.epoll.max_user_watches"
+sys_file="/proc/sys/fs/epoll/max_user_watches"
+
+. tst_test.sh
+
+setup()
+{
+	orig_value=$(cat "$sys_file")
+}
+
+do_test()
+{
+	tst_res TINFO "trying to set $sys_name=-1"
+
+	sysctl -w -q $sys_name=-1 2>&1 | grep -q "Invalid argument"
+
+	if [ $? -eq 0 ]; then
+		tst_res TPASS "sysctl returns EINVAL when passing a negative value"
+	else
+		tst_res TFAIL "sysctl doesn't return EINVAL when passing a negative value"
+	fi
+
+	new_value=$(cat "$sys_file")
+	if [ $orig_value -eq $new_value ]; then
+		tst_res TPASS "the interface value doesn't change when passing a negative value"
+	else
+		tst_res TFAIL "the interface value changes when passing a negative value"
+		sysctl -w -q $sys_name=$orig_value
+	fi
+}
+
+tst_run