Linux-Fsdevel Archive on lore.kernel.org
 help / Atom feed
* Re: [PATCH 1/2] test_sysctl: add tests for >32-bit values written to 32-bit integers
@ 2019-02-06 19:52 Luis Chamberlain
  0 siblings, 0 replies; 2+ messages in thread
From: Luis Chamberlain @ 2019-02-06 19:52 UTC (permalink / raw)
  To: Zev Weiss
  Cc: Kees Cook, linux-kernel, linux-fsdevel, Shuah Khan,
	linux-kselftest, akpm, yzaikin, brendanhiggins

Thanks for the patches, please include akpm@linux-foundation.org in the
future, as we can merge the changes through Andrew as well.

Also please Cc yzaikin@google.com, brendanhiggins@google.com in follow
ups for now.  They are looking at the sysctl testing code as well.

Some feedback below:

In-Reply-To: <20181227111231.12912-2-zev@bewilderbeest.net>

On Thu, Dec 27, 2018 at 05:12:29AM -0600, Zev Weiss wrote:
> +run_wideint_tests()
> +{
> +	# check negative and positive 64-bit values, with and without
> +	# bits set in the lower 31, and with and without bit 31 (sign
> +	# bit of a 32-bit int) set.  None of these are representable
> +	# in 32 bits, and hence all should fail.
> +	check_failure 0x0000010000000000
> +	check_failure 0x0000010080000000
> +	check_failure 0x000001ff7fffffff
> +	check_failure 0x000001ffffffffff
> +	check_failure 0xffffffff7fffffff


> +	check_failure 0xffffffffffffffff

This s64 version of -1

> +	check_failure 0xffffff0000000000
> +	check_failure 0xffffff0080000000
> +}

It was still unclear from the comments and manually looking at the
values why they are clear candidates to always test from all respective
64-bit values. A comment per each would be useful.

  Luis

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH 1/2] test_sysctl: add tests for >32-bit values written to 32-bit integers
  2018-12-27 11:12 [PATCH 0/2] sysctl: fix range-checking in do_proc_dointvec_minmax_conv() Zev Weiss
@ 2018-12-27 11:12 ` Zev Weiss
  0 siblings, 0 replies; 2+ messages in thread
From: Zev Weiss @ 2018-12-27 11:12 UTC (permalink / raw)
  To: Luis Chamberlain, Kees Cook
  Cc: linux-kernel, linux-fsdevel, Zev Weiss, Shuah Khan, linux-kselftest

At present this exposes a bug in do_proc_dointvec_minmax_conv() (it
fails to check for values that are too wide to fit in an int).

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 tools/testing/selftests/sysctl/sysctl.sh | 37 ++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 584eb8ea780a..a7d0da25975c 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -290,6 +290,40 @@ run_numerictests()
 	test_rc
 }
 
+check_failure()
+{
+	echo -n "Testing that $1 fails as expected..."
+	reset_vals
+	TEST_STR="$1"
+	orig="$(cat $TARGET)"
+	echo -n "$TEST_STR" > $TARGET 2> /dev/null
+
+	# write should fail and $TARGET should retain its original value
+	if [ $? = 0 ] || [ "$(cat $TARGET)" != "$orig" ]; then
+		echo "FAIL" >&2
+		rc=1
+	else
+		echo "ok"
+	fi
+	test_rc
+}
+
+run_wideint_tests()
+{
+	# check negative and positive 64-bit values, with and without
+	# bits set in the lower 31, and with and without bit 31 (sign
+	# bit of a 32-bit int) set.  None of these are representable
+	# in 32 bits, and hence all should fail.
+	check_failure 0x0000010000000000
+	check_failure 0x0000010080000000
+	check_failure 0x000001ff7fffffff
+	check_failure 0x000001ffffffffff
+	check_failure 0xffffffff7fffffff
+	check_failure 0xffffffffffffffff
+	check_failure 0xffffff0000000000
+	check_failure 0xffffff0080000000
+}
+
 # Your test must accept digits 3 and 4 to use this
 run_limit_digit()
 {
@@ -556,6 +590,7 @@ sysctl_test_0001()
 	TEST_STR=$(( $ORIG + 1 ))
 
 	run_numerictests
+	run_wideint_tests
 	run_limit_digit
 }
 
@@ -580,6 +615,7 @@ sysctl_test_0003()
 	TEST_STR=$(( $ORIG + 1 ))
 
 	run_numerictests
+	run_wideint_tests
 	run_limit_digit
 	run_limit_digit_int
 }
@@ -592,6 +628,7 @@ sysctl_test_0004()
 	TEST_STR=$(( $ORIG + 1 ))
 
 	run_numerictests
+	run_wideint_tests
 	run_limit_digit
 	run_limit_digit_uint
 }
-- 
2.20.1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-06 19:52 [PATCH 1/2] test_sysctl: add tests for >32-bit values written to 32-bit integers Luis Chamberlain
  -- strict thread matches above, loose matches on Subject: below --
2018-12-27 11:12 [PATCH 0/2] sysctl: fix range-checking in do_proc_dointvec_minmax_conv() Zev Weiss
2018-12-27 11:12 ` [PATCH 1/2] test_sysctl: add tests for >32-bit values written to 32-bit integers Zev Weiss

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org linux-fsdevel@archiver.kernel.org
	public-inbox-index linux-fsdevel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox