All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: akpm@linux-foundation.org, keescook@chromium.org
Cc: sandeen@redhat.com, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, Eric Sandeen <sandeen@sandeen.net>,
	Luis Chamberlain <mcgrof@kernel.org>
Subject: [PATCH 5/6] test_sysctl: add proc_do_large_bitmap() test case
Date: Wed, 20 Mar 2019 22:28:30 +0000	[thread overview]
Message-ID: <20190320222831.8243-6-mcgrof@kernel.org> (raw)
In-Reply-To: <20190320222831.8243-1-mcgrof@kernel.org>

From: Eric Sandeen <sandeen@sandeen.net>

The kernel has only two users of proc_do_large_bitmap(), the kernel
CPU watchdog, and the ip_local_reserved_ports. Refer to watchdog_cpumask
and ip_local_reserved_ports in Documentation for further details on
these. When you input a large buffer into these, when it is larger than
PAGE_SIZE - 1, the input data gets misparsed, and the user get
incorrectly informed that the desired input value was set. This commit
implements a test which mimics and exploits that use case, it uses a
bitmap size, as in the watchdog case. The bitmap is used to test the
bitmap proc handler, proc_do_large_bitmap().

The next commit fixes this issue.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
[mcgrof: use new target description for backward compatibility]
[mcgrof: augment test number to 50, ran into issues with bash
         string comparisons when testing up to 50 cases.]
[mcgrof: introduce and use verify_diff_proc_file() to use diff]
[mcgrof: use mktemp for tmp file]
[mcgrof: merge shell test and C code]
[mcgrof: commit log love]
[mcgrof: export proc_do_large_bitmap() to allow for the test
[mcgrof: check for the return value when writing to the proc file]
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 kernel/sysctl.c                          |  1 +
 lib/test_sysctl.c                        | 18 +++++-
 tools/testing/selftests/sysctl/sysctl.sh | 81 +++++++++++++++++++++++-
 3 files changed, 97 insertions(+), 3 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index b3df3ab7ac28..e1a8d785b839 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -3263,6 +3263,7 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
 	kfree(tmp_bitmap);
 	return err;
 }
+EXPORT_SYMBOL_GPL(proc_do_large_bitmap);
 
 #else /* CONFIG_PROC_SYSCTL */
 
diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
index 3dd801c1c85b..566dad3f4196 100644
--- a/lib/test_sysctl.c
+++ b/lib/test_sysctl.c
@@ -47,6 +47,9 @@ struct test_sysctl_data {
 	unsigned int uint_0001;
 
 	char string_0001[65];
+
+#define SYSCTL_TEST_BITMAP_SIZE	65536
+	unsigned long *bitmap_0001;
 };
 
 static struct test_sysctl_data test_data = {
@@ -102,6 +105,13 @@ static struct ctl_table test_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dostring,
 	},
+	{
+		.procname	= "bitmap_0001",
+		.data		= &test_data.bitmap_0001,
+		.maxlen		= SYSCTL_TEST_BITMAP_SIZE,
+		.mode		= 0644,
+		.proc_handler	= proc_do_large_bitmap,
+	},
 	{ }
 };
 
@@ -129,15 +139,21 @@ static struct ctl_table_header *test_sysctl_header;
 
 static int __init test_sysctl_init(void)
 {
+	test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL);
+	if (!test_data.bitmap_0001)
+		return -ENOMEM;
 	test_sysctl_header = register_sysctl_table(test_sysctl_root_table);
-	if (!test_sysctl_header)
+	if (!test_sysctl_header) {
+		kfree(test_data.bitmap_0001);
 		return -ENOMEM;
+	}
 	return 0;
 }
 late_initcall(test_sysctl_init);
 
 static void __exit test_sysctl_exit(void)
 {
+	kfree(test_data.bitmap_0001);
 	if (test_sysctl_header)
 		unregister_sysctl_table(test_sysctl_header);
 }
diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 4eb019068e24..6a970b127c9b 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -38,6 +38,7 @@ ALL_TESTS="$ALL_TESTS 0002:1:1:string_0001"
 ALL_TESTS="$ALL_TESTS 0003:1:1:int_0002"
 ALL_TESTS="$ALL_TESTS 0004:1:1:uint_0001"
 ALL_TESTS="$ALL_TESTS 0005:3:1:int_0003"
+ALL_TESTS="$ALL_TESTS 0006:50:1:bitmap_0001"
 
 test_modprobe()
 {
@@ -150,6 +151,9 @@ reset_vals()
 		string_0001)
 			VAL="(none)"
 			;;
+		bitmap_0001)
+			VAL=""
+			;;
 		*)
 			;;
 	esac
@@ -180,6 +184,22 @@ verify()
 	return 0
 }
 
+# proc files get read a page at a time, which can confuse diff,
+# and get you incorrect results on proc files with long data. To use
+# diff against them you must first extract the output to a file, and
+# then compare against that file.
+verify_diff_proc_file()
+{
+	TMP_DUMP_FILE=$(mktemp)
+	cat $1 > $TMP_DUMP_FILE
+
+	if ! diff -w -q $TMP_DUMP_FILE $2; then
+		return 1
+	else
+		return 0
+	fi
+}
+
 verify_diff_w()
 {
 	echo "$TEST_STR" | diff -q -w -u - $1 > /dev/null
@@ -615,6 +635,55 @@ target_exists()
 	return 1
 }
 
+run_bitmaptest() {
+	# Total length of bitmaps string to use, a bit under
+	# the maximum input size of the test node
+	LENGTH=$((RANDOM % 65000))
+
+	# First bit to set
+	BIT=$((RANDOM % 1024))
+
+	# String containing our list of bits to set
+	TEST_STR=$BIT
+
+	# build up the string
+	while [ "${#TEST_STR}" -le "$LENGTH" ]; do
+		# Make sure next entry is discontiguous,
+		# skip ahead at least 2
+		BIT=$((BIT + $((2 + RANDOM % 10))))
+
+		# Add new bit to the list
+		TEST_STR="${TEST_STR},${BIT}"
+
+		# Randomly make it a range
+		if [ "$((RANDOM % 2))" -eq "1" ]; then
+			RANGE_END=$((BIT + $((1 + RANDOM % 10))))
+			TEST_STR="${TEST_STR}-${RANGE_END}"
+			BIT=$RANGE_END
+		fi
+	done
+
+	echo -n "Checking bitmap handler... "
+	TEST_FILE=$(mktemp)
+	echo -n "$TEST_STR" > $TEST_FILE
+
+	cat $TEST_FILE > $TARGET 2> /dev/null
+	if [ $? -ne 0 ]; then
+		echo "FAIL" >&2
+		rc=1
+		test_rc
+	fi
+
+	if ! verify_diff_proc_file "$TARGET" "$TEST_FILE"; then
+		echo "FAIL" >&2
+		rc=1
+	else
+		echo "ok"
+		rc=0
+	fi
+	test_rc
+}
+
 sysctl_test_0001()
 {
 	TARGET="${SYSCTL}/$(get_test_target 0001)"
@@ -675,6 +744,14 @@ sysctl_test_0005()
 	run_limit_digit_int_array
 }
 
+sysctl_test_0006()
+{
+	TARGET="${SYSCTL}/bitmap_0001"
+	reset_vals
+	ORIG=""
+	run_bitmaptest
+}
+
 list_tests()
 {
 	echo "Test ID list:"
@@ -688,6 +765,7 @@ list_tests()
 	echo "0003 x $(get_test_count 0003) - tests proc_dointvec()"
 	echo "0004 x $(get_test_count 0004) - tests proc_douintvec()"
 	echo "0005 x $(get_test_count 0005) - tests proc_douintvec() array"
+	echo "0006 x $(get_test_count 0006) - tests proc_do_large_bitmap()"
 }
 
 usage()
@@ -761,8 +839,7 @@ function run_all_tests()
 		ENABLED=$(get_test_enabled $TEST_ID)
 		TEST_COUNT=$(get_test_count $TEST_ID)
 		TEST_TARGET=$(get_test_target $TEST_ID)
-		target_exists $TEST_TARGET $TEST_ID
-		if [ $? -ne 1 ]; then
+		if target_exists $TEST_TARGET $TEST_ID; then
 			continue
 		fi
 		if [[ $ENABLED -eq "1" ]]; then
-- 
2.18.0


  parent reply	other threads:[~2019-03-20 22:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-20 22:28 [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Luis Chamberlain
2019-03-20 22:28 ` [PATCH 1/6] test_sysctl: remove superfluous test_reqs() Luis Chamberlain
2019-03-20 22:28 ` [PATCH 2/6] test_sysctl: load module before testing for it Luis Chamberlain
2019-03-20 22:28 ` [PATCH 3/6] test_sysctl: ignore diff output on verify_diff_w() Luis Chamberlain
2019-03-20 22:28 ` [PATCH 4/6] test_sysctl: allow graceful use on older kernels Luis Chamberlain
2019-03-20 22:28 ` Luis Chamberlain [this message]
2019-03-20 22:28 ` [PATCH 6/6] sysctl: Fix proc_do_large_bitmap for large input buffers Luis Chamberlain
2019-03-21 16:42 ` [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Kees Cook
2019-04-24 17:42   ` Eric Sandeen
2019-04-24 19:05     ` Kees Cook
2019-03-21 17:13 ` Eric Sandeen
2019-03-21 19:18   ` Eric Sandeen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190320222831.8243-6-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --cc=sandeen@sandeen.net \
    --subject='Re: [PATCH 5/6] test_sysctl: add proc_do_large_bitmap() test case' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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.