All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix
@ 2019-03-20 22:28 Luis Chamberlain
  2019-03-20 22:28 ` [PATCH 1/6] test_sysctl: remove superfluous test_reqs() Luis Chamberlain
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Luis Chamberlain @ 2019-03-20 22:28 UTC (permalink / raw)
  To: akpm, keescook; +Cc: sandeen, linux-fsdevel, linux-kernel, Luis Chamberlain

Andrew, Kees,

Eric sent a fix out for proc_do_large_bitmap() last month for when
using a large input buffer. After patch review a test case for the issue
was built and submitted. I noticed there were a few issues with the
tests, but instead of just asking Eric to address them I've taken
care of them and ammended the commit where necessary. There's a
few issues he reported which I also address and fix in this series.

Since we *do* expect users of these scripts to also use them on older
kernels, I've also addressed not breaking calling the script for them,
and gives us an easy way to easily extend our tests cases for future
kernels as well.

Before anyone considers these for stable as minor fixes, I'd recommend
we also address the discrepancy on the read side of things: modify the
test script to use diff against the target file instead of using the
temp file.

Eric Sandeen (2):
  test_sysctl: add proc_do_large_bitmap() test case
  sysctl: Fix proc_do_large_bitmap for large input buffers

Luis Chamberlain (4):
  test_sysctl: remove superfluous test_reqs()
  test_sysctl: load module before testing for it
  test_sysctl: ignore diff output on verify_diff_w()
  test_sysctl: allow graceful use on older kernels

 kernel/sysctl.c                          |  30 ++++-
 lib/test_sysctl.c                        |  18 ++-
 tools/testing/selftests/sysctl/sysctl.sh | 161 +++++++++++++++++++----
 3 files changed, 178 insertions(+), 31 deletions(-)

-- 
2.18.0


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

* [PATCH 1/6] test_sysctl: remove superfluous test_reqs()
  2019-03-20 22:28 [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Luis Chamberlain
@ 2019-03-20 22:28 ` Luis Chamberlain
  2019-03-20 22:28 ` [PATCH 2/6] test_sysctl: load module before testing for it Luis Chamberlain
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Luis Chamberlain @ 2019-03-20 22:28 UTC (permalink / raw)
  To: akpm, keescook; +Cc: sandeen, linux-fsdevel, linux-kernel, Luis Chamberlain

We already call test_reqs(), no need to call it twice.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 tools/testing/selftests/sysctl/sysctl.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 780ce7123374..87df7e52a97a 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -675,8 +675,6 @@ list_tests()
 	echo "0005 x $(get_test_count 0005) - tests proc_douintvec() array"
 }
 
-test_reqs
-
 usage()
 {
 	NUM_TESTS=$(grep -o ' ' <<<"$ALL_TESTS" | grep -c .)
-- 
2.18.0


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

* [PATCH 2/6] test_sysctl: load module before testing for it
  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 ` Luis Chamberlain
  2019-03-20 22:28 ` [PATCH 3/6] test_sysctl: ignore diff output on verify_diff_w() Luis Chamberlain
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Luis Chamberlain @ 2019-03-20 22:28 UTC (permalink / raw)
  To: akpm, keescook; +Cc: sandeen, linux-fsdevel, linux-kernel, Luis Chamberlain

Currently the test script checks for the existance of the
sysctl test module's directory path prior to loading it.
We must first try to load the module prior to checking for
that path. This fixes the order for the load / test.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 tools/testing/selftests/sysctl/sysctl.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 87df7e52a97a..e0c8404da6b0 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -823,8 +823,8 @@ function parse_args()
 test_reqs
 allow_user_defaults
 check_production_sysctl_writes_strict
-test_modprobe
 load_req_mod
+test_modprobe
 
 trap "test_finish" EXIT
 
-- 
2.18.0


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

* [PATCH 3/6] test_sysctl: ignore diff output on verify_diff_w()
  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 ` Luis Chamberlain
  2019-03-20 22:28 ` [PATCH 4/6] test_sysctl: allow graceful use on older kernels Luis Chamberlain
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Luis Chamberlain @ 2019-03-20 22:28 UTC (permalink / raw)
  To: akpm, keescook; +Cc: sandeen, linux-fsdevel, linux-kernel, Luis Chamberlain

When verify_diff_w() is used we care about the result, not
the verbose output, and although we use -q, that still
gives us a chatty message about if the files differ or not.
Since verify_diff_w() uses stdinput the chatty message says
whether or not "-" matches the target file, and this just
seems rather odd. Better to just ignore that messsage all
together, what we really care about i sthe results, the
return value and we check for that.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 tools/testing/selftests/sysctl/sysctl.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index e0c8404da6b0..f51987d0d32d 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -179,7 +179,7 @@ verify()
 
 verify_diff_w()
 {
-	echo "$TEST_STR" | diff -q -w -u - $1
+	echo "$TEST_STR" | diff -q -w -u - $1 > /dev/null
 	return $?
 }
 
-- 
2.18.0


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

* [PATCH 4/6] test_sysctl: allow graceful use on older kernels
  2019-03-20 22:28 [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Luis Chamberlain
                   ` (2 preceding siblings ...)
  2019-03-20 22:28 ` [PATCH 3/6] test_sysctl: ignore diff output on verify_diff_w() Luis Chamberlain
@ 2019-03-20 22:28 ` Luis Chamberlain
  2019-03-20 22:28 ` [PATCH 5/6] test_sysctl: add proc_do_large_bitmap() test case Luis Chamberlain
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Luis Chamberlain @ 2019-03-20 22:28 UTC (permalink / raw)
  To: akpm, keescook; +Cc: sandeen, linux-fsdevel, linux-kernel, Luis Chamberlain

On old kernels older new test knobs implemented on the test_sysctl
module may not be available. This is expected, and the selftests test
scripts should be able to run without failures on older kernels.

Generalize a solution so that we test for each required test target
file for each test by requiring each test description to annotate their
respective test target file. If the target file does not exist, we skip
the test gracefully.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 tools/testing/selftests/sysctl/sysctl.sh | 78 ++++++++++++++++--------
 1 file changed, 53 insertions(+), 25 deletions(-)

diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index f51987d0d32d..4eb019068e24 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -24,19 +24,20 @@ TEST_FILE=$(mktemp)
 
 # This represents
 #
-# TEST_ID:TEST_COUNT:ENABLED
+# TEST_ID:TEST_COUNT:ENABLED:TARGET
 #
 # TEST_ID: is the test id number
 # TEST_COUNT: number of times we should run the test
 # ENABLED: 1 if enabled, 0 otherwise
+# TARGET: test target file required on the test_sysctl module
 #
 # Once these are enabled please leave them as-is. Write your own test,
 # we have tons of space.
-ALL_TESTS="0001:1:1"
-ALL_TESTS="$ALL_TESTS 0002:1:1"
-ALL_TESTS="$ALL_TESTS 0003:1:1"
-ALL_TESTS="$ALL_TESTS 0004:1:1"
-ALL_TESTS="$ALL_TESTS 0005:3:1"
+ALL_TESTS="0001:1:1:int_0001"
+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"
 
 test_modprobe()
 {
@@ -157,8 +158,10 @@ reset_vals()
 
 set_orig()
 {
-	if [ ! -z $TARGET ]; then
-		echo "${ORIG}" > "${TARGET}"
+	if [ ! -z $TARGET ] && [ ! -z $ORIG ]; then
+		if [ -f ${TARGET} ]; then
+			echo "${ORIG}" > "${TARGET}"
+		fi
 	fi
 }
 
@@ -600,9 +603,21 @@ run_stringtests()
 	test_rc
 }
 
+target_exists()
+{
+	TARGET="${SYSCTL}/$1"
+	TEST_ID="$2"
+
+	if [ ! -f ${TARGET} ] ; then
+		echo "Target for test $TEST_ID: $TARGET not exist, skipping test ..."
+		return 0
+	fi
+	return 1
+}
+
 sysctl_test_0001()
 {
-	TARGET="${SYSCTL}/int_0001"
+	TARGET="${SYSCTL}/$(get_test_target 0001)"
 	reset_vals
 	ORIG=$(cat "${TARGET}")
 	TEST_STR=$(( $ORIG + 1 ))
@@ -614,7 +629,7 @@ sysctl_test_0001()
 
 sysctl_test_0002()
 {
-	TARGET="${SYSCTL}/string_0001"
+	TARGET="${SYSCTL}/$(get_test_target 0002)"
 	reset_vals
 	ORIG=$(cat "${TARGET}")
 	TEST_STR="Testing sysctl"
@@ -627,7 +642,7 @@ sysctl_test_0002()
 
 sysctl_test_0003()
 {
-	TARGET="${SYSCTL}/int_0002"
+	TARGET="${SYSCTL}/$(get_test_target 0003)"
 	reset_vals
 	ORIG=$(cat "${TARGET}")
 	TEST_STR=$(( $ORIG + 1 ))
@@ -640,7 +655,7 @@ sysctl_test_0003()
 
 sysctl_test_0004()
 {
-	TARGET="${SYSCTL}/uint_0001"
+	TARGET="${SYSCTL}/$(get_test_target 0004)"
 	reset_vals
 	ORIG=$(cat "${TARGET}")
 	TEST_STR=$(( $ORIG + 1 ))
@@ -653,7 +668,7 @@ sysctl_test_0004()
 
 sysctl_test_0005()
 {
-	TARGET="${SYSCTL}/int_0003"
+	TARGET="${SYSCTL}/$(get_test_target 0005)"
 	reset_vals
 	ORIG=$(cat "${TARGET}")
 
@@ -722,25 +737,36 @@ function get_test_count()
 {
 	test_num $1
 	TEST_DATA=$(echo $ALL_TESTS | awk '{print $'$1'}')
-	LAST_TWO=${TEST_DATA#*:*}
-	echo ${LAST_TWO%:*}
+	echo ${TEST_DATA} | awk -F":" '{print $2}'
 }
 
 function get_test_enabled()
 {
 	test_num $1
 	TEST_DATA=$(echo $ALL_TESTS | awk '{print $'$1'}')
-	echo ${TEST_DATA#*:*:}
+	echo ${TEST_DATA} | awk -F":" '{print $3}'
+}
+
+function get_test_target()
+{
+	test_num $1
+	TEST_DATA=$(echo $ALL_TESTS | awk '{print $'$1'}')
+	echo ${TEST_DATA} | awk -F":" '{print $4}'
 }
 
 function run_all_tests()
 {
 	for i in $ALL_TESTS ; do
-		TEST_ID=${i%:*:*}
+		TEST_ID=${i%:*:*:*}
 		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
+			continue
+		fi
 		if [[ $ENABLED -eq "1" ]]; then
-			test_case $TEST_ID $TEST_COUNT
+			test_case $TEST_ID $TEST_COUNT $TEST_TARGET
 		fi
 	done
 }
@@ -773,12 +799,14 @@ function watch_case()
 
 function test_case()
 {
-	NUM_TESTS=$DEFAULT_NUM_TESTS
-	if [ $# -eq 2 ]; then
-		NUM_TESTS=$2
-	fi
+	NUM_TESTS=$2
 
 	i=0
+
+	if target_exists $3 $1; then
+		continue
+	fi
+
 	while [ $i -lt $NUM_TESTS ]; do
 		test_num $1
 		watch_log $i ${TEST_NAME}_test_$1 noclear
@@ -801,15 +829,15 @@ function parse_args()
 		elif [[ "$1" = "-t" ]]; then
 			shift
 			test_num $1
-			test_case $1 $(get_test_count $1)
+			test_case $1 $(get_test_count $1) $(get_test_target $1)
 		elif [[ "$1" = "-c" ]]; then
 			shift
 			test_num $1
 			test_num $2
-			test_case $1 $2
+			test_case $1 $2 $(get_test_target $1)
 		elif [[ "$1" = "-s" ]]; then
 			shift
-			test_case $1 1
+			test_case $1 1 $(get_test_target $1)
 		elif [[ "$1" = "-l" ]]; then
 			list_tests
 		elif [[ "$1" = "-h" || "$1" = "--help" ]]; then
-- 
2.18.0


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

* [PATCH 5/6] test_sysctl: add proc_do_large_bitmap() test case
  2019-03-20 22:28 [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Luis Chamberlain
                   ` (3 preceding siblings ...)
  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
  2019-03-20 22:28 ` [PATCH 6/6] sysctl: Fix proc_do_large_bitmap for large input buffers Luis Chamberlain
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Luis Chamberlain @ 2019-03-20 22:28 UTC (permalink / raw)
  To: akpm, keescook
  Cc: sandeen, linux-fsdevel, linux-kernel, Eric Sandeen, Luis Chamberlain

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


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

* [PATCH 6/6] sysctl: Fix proc_do_large_bitmap for large input buffers
  2019-03-20 22:28 [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Luis Chamberlain
                   ` (4 preceding siblings ...)
  2019-03-20 22:28 ` [PATCH 5/6] test_sysctl: add proc_do_large_bitmap() test case Luis Chamberlain
@ 2019-03-20 22:28 ` Luis Chamberlain
  2019-03-21 16:42 ` [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Kees Cook
  2019-03-21 17:13 ` Eric Sandeen
  7 siblings, 0 replies; 12+ messages in thread
From: Luis Chamberlain @ 2019-03-20 22:28 UTC (permalink / raw)
  To: akpm, keescook; +Cc: sandeen, linux-fsdevel, linux-kernel, Luis Chamberlain

From: Eric Sandeen <sandeen@redhat.com>

Today, proc_do_large_bitmap() truncates a large write input buffer
to PAGE_SIZE - 1, which may result in misparsed numbers at the
(truncated) end of the buffer.  Further, it fails to notify the caller
that the buffer was truncated, so it doesn't get called iteratively
to finish the entire input buffer.

Tell the caller if there's more work to do by adding the skipped
amount back to left/*lenp before returning.

To fix the misparsing, reset the position if we have completely
consumed a truncated buffer (or if just one char is left, which
may be a "-" in a range), and ask the caller to come back for
more.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 kernel/sysctl.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e1a8d785b839..ddc6c717355d 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -3159,9 +3159,13 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
 
 	if (write) {
 		char *kbuf, *p;
+		size_t skipped = 0;
 
-		if (left > PAGE_SIZE - 1)
+		if (left > PAGE_SIZE - 1) {
 			left = PAGE_SIZE - 1;
+			/* How much of the buffer we'll skip this pass */
+			skipped = *lenp - left;
+		}
 
 		p = kbuf = memdup_user_nul(buffer, left);
 		if (IS_ERR(kbuf))
@@ -3178,9 +3182,22 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
 		while (!err && left) {
 			unsigned long val_a, val_b;
 			bool neg;
+			size_t saved_left;
 
+			/* In case we stop parsing mid-number, we can reset */
+			saved_left = left;
 			err = proc_get_long(&p, &left, &val_a, &neg, tr_a,
 					     sizeof(tr_a), &c);
+			/*
+			 * If we consumed the entirety of a truncated buffer or
+			 * only one char is left (may be a "-"), then stop here,
+			 * reset, & come back for more.
+			 */
+			if ((left <= 1) && skipped) {
+				left = saved_left;
+				break;
+			}
+
 			if (err)
 				break;
 			if (val_a >= bitmap_len || neg) {
@@ -3198,6 +3215,15 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
 				err = proc_get_long(&p, &left, &val_b,
 						     &neg, tr_b, sizeof(tr_b),
 						     &c);
+				/*
+				 * If we consumed all of a truncated buffer or
+				 * then stop here, reset, & come back for more.
+				 */
+				if (!left && skipped) {
+					left = saved_left;
+					break;
+				}
+
 				if (err)
 					break;
 				if (val_b >= bitmap_len || neg ||
@@ -3216,6 +3242,7 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
 			proc_skip_char(&p, &left, '\n');
 		}
 		kfree(kbuf);
+		left += skipped;
 	} else {
 		unsigned long bit_a, bit_b = 0;
 
-- 
2.18.0


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

* Re: [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix
  2019-03-20 22:28 [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Luis Chamberlain
                   ` (5 preceding siblings ...)
  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 ` Kees Cook
  2019-04-24 17:42   ` Eric Sandeen
  2019-03-21 17:13 ` Eric Sandeen
  7 siblings, 1 reply; 12+ messages in thread
From: Kees Cook @ 2019-03-21 16:42 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Andrew Morton, Eric Sandeen, linux-fsdevel, LKML

On Wed, Mar 20, 2019 at 3:28 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> Andrew, Kees,
>
> Eric sent a fix out for proc_do_large_bitmap() last month for when
> using a large input buffer. After patch review a test case for the issue
> was built and submitted. I noticed there were a few issues with the
> tests, but instead of just asking Eric to address them I've taken
> care of them and ammended the commit where necessary. There's a
> few issues he reported which I also address and fix in this series.
>
> Since we *do* expect users of these scripts to also use them on older
> kernels, I've also addressed not breaking calling the script for them,
> and gives us an easy way to easily extend our tests cases for future
> kernels as well.
>
> Before anyone considers these for stable as minor fixes, I'd recommend
> we also address the discrepancy on the read side of things: modify the
> test script to use diff against the target file instead of using the
> temp file.
>
> Eric Sandeen (2):
>   test_sysctl: add proc_do_large_bitmap() test case
>   sysctl: Fix proc_do_large_bitmap for large input buffers
>
> Luis Chamberlain (4):
>   test_sysctl: remove superfluous test_reqs()
>   test_sysctl: load module before testing for it
>   test_sysctl: ignore diff output on verify_diff_w()
>   test_sysctl: allow graceful use on older kernels

Thanks for collecting and updating these!

Acked-by: Kees Cook <keescook@chromium.org>

Andrew, can you carry these?

-Kees

>
>  kernel/sysctl.c                          |  30 ++++-
>  lib/test_sysctl.c                        |  18 ++-
>  tools/testing/selftests/sysctl/sysctl.sh | 161 +++++++++++++++++++----
>  3 files changed, 178 insertions(+), 31 deletions(-)
>
> --
> 2.18.0
>


-- 
Kees Cook

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

* Re: [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix
  2019-03-20 22:28 [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Luis Chamberlain
                   ` (6 preceding siblings ...)
  2019-03-21 16:42 ` [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix Kees Cook
@ 2019-03-21 17:13 ` Eric Sandeen
  2019-03-21 19:18   ` Eric Sandeen
  7 siblings, 1 reply; 12+ messages in thread
From: Eric Sandeen @ 2019-03-21 17:13 UTC (permalink / raw)
  To: Luis Chamberlain, akpm, keescook; +Cc: linux-fsdevel, linux-kernel

On 3/20/19 5:28 PM, Luis Chamberlain wrote:
> Andrew, Kees,
> 
> Eric sent a fix out for proc_do_large_bitmap() last month for when
> using a large input buffer. After patch review a test case for the issue
> was built and submitted. I noticed there were a few issues with the
> tests, but instead of just asking Eric to address them I've taken
> care of them and ammended the commit where necessary. There's a
> few issues he reported which I also address and fix in this series.
> 
> Since we *do* expect users of these scripts to also use them on older
> kernels, I've also addressed not breaking calling the script for them,
> and gives us an easy way to easily extend our tests cases for future
> kernels as well.
> 
> Before anyone considers these for stable as minor fixes, I'd recommend
> we also address the discrepancy on the read side of things: modify the
> test script to use diff against the target file instead of using the
> temp file.
> 
> Eric Sandeen (2):
>   test_sysctl: add proc_do_large_bitmap() test case
>   sysctl: Fix proc_do_large_bitmap for large input buffers

Isn't this missing:

[PATCH] sysctl: add proc_do_large_bitmap test node

?

> Luis Chamberlain (4):
>   test_sysctl: remove superfluous test_reqs()
>   test_sysctl: load module before testing for it
>   test_sysctl: ignore diff output on verify_diff_w()
>   test_sysctl: allow graceful use on older kernels
> 
>  kernel/sysctl.c                          |  30 ++++-
>  lib/test_sysctl.c                        |  18 ++-
>  tools/testing/selftests/sysctl/sysctl.sh | 161 +++++++++++++++++++----
>  3 files changed, 178 insertions(+), 31 deletions(-)
> 


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

* Re: [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix
  2019-03-21 17:13 ` Eric Sandeen
@ 2019-03-21 19:18   ` Eric Sandeen
  0 siblings, 0 replies; 12+ messages in thread
From: Eric Sandeen @ 2019-03-21 19:18 UTC (permalink / raw)
  To: Eric Sandeen, Luis Chamberlain, akpm, keescook
  Cc: linux-fsdevel, linux-kernel



On 3/21/19 12:13 PM, Eric Sandeen wrote:
> On 3/20/19 5:28 PM, Luis Chamberlain wrote:
>> Andrew, Kees,
>>
>> Eric sent a fix out for proc_do_large_bitmap() last month for when
>> using a large input buffer. After patch review a test case for the issue
>> was built and submitted. I noticed there were a few issues with the
>> tests, but instead of just asking Eric to address them I've taken
>> care of them and ammended the commit where necessary. There's a
>> few issues he reported which I also address and fix in this series.
>>
>> Since we *do* expect users of these scripts to also use them on older
>> kernels, I've also addressed not breaking calling the script for them,
>> and gives us an easy way to easily extend our tests cases for future
>> kernels as well.
>>
>> Before anyone considers these for stable as minor fixes, I'd recommend
>> we also address the discrepancy on the read side of things: modify the
>> test script to use diff against the target file instead of using the
>> temp file.
>>
>> Eric Sandeen (2):
>>   test_sysctl: add proc_do_large_bitmap() test case
>>   sysctl: Fix proc_do_large_bitmap for large input buffers
> 
> Isn't this missing:
> 
> [PATCH] sysctl: add proc_do_large_bitmap test node

Oh, I see, you rolled it into the test patch.  that wasn't clear...

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

* Re: [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Eric Sandeen @ 2019-04-24 17:42 UTC (permalink / raw)
  To: Kees Cook, Luis Chamberlain
  Cc: Andrew Morton, Eric Sandeen, linux-fsdevel, LKML

On 3/21/19 11:42 AM, Kees Cook wrote:
> On Wed, Mar 20, 2019 at 3:28 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>>
>> Andrew, Kees,
>>
>> Eric sent a fix out for proc_do_large_bitmap() last month for when
>> using a large input buffer. After patch review a test case for the issue
>> was built and submitted. I noticed there were a few issues with the
>> tests, but instead of just asking Eric to address them I've taken
>> care of them and ammended the commit where necessary. There's a
>> few issues he reported which I also address and fix in this series.
>>
>> Since we *do* expect users of these scripts to also use them on older
>> kernels, I've also addressed not breaking calling the script for them,
>> and gives us an easy way to easily extend our tests cases for future
>> kernels as well.
>>
>> Before anyone considers these for stable as minor fixes, I'd recommend
>> we also address the discrepancy on the read side of things: modify the
>> test script to use diff against the target file instead of using the
>> temp file.
>>
>> Eric Sandeen (2):
>>   test_sysctl: add proc_do_large_bitmap() test case
>>   sysctl: Fix proc_do_large_bitmap for large input buffers
>>
>> Luis Chamberlain (4):
>>   test_sysctl: remove superfluous test_reqs()
>>   test_sysctl: load module before testing for it
>>   test_sysctl: ignore diff output on verify_diff_w()
>>   test_sysctl: allow graceful use on older kernels
> 
> Thanks for collecting and updating these!
> 
> Acked-by: Kees Cook <keescook@chromium.org>
> 
> Andrew, can you carry these?

Ping?  This seems to have never made it off the list into anybody's
tree.

Thanks,
-Eric

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

* Re: [PATCH 0/6] sysctl: add pending proc_do_large_bitmap fix
  2019-04-24 17:42   ` Eric Sandeen
@ 2019-04-24 19:05     ` Kees Cook
  0 siblings, 0 replies; 12+ messages in thread
From: Kees Cook @ 2019-04-24 19:05 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Luis Chamberlain, Andrew Morton, Eric Sandeen, linux-fsdevel, LKML

On Wed, Apr 24, 2019 at 10:42 AM Eric Sandeen <sandeen@sandeen.net> wrote:
>
> On 3/21/19 11:42 AM, Kees Cook wrote:
> > On Wed, Mar 20, 2019 at 3:28 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >>
> >> Andrew, Kees,
> >>
> >> Eric sent a fix out for proc_do_large_bitmap() last month for when
> >> using a large input buffer. After patch review a test case for the issue
> >> was built and submitted. I noticed there were a few issues with the
> >> tests, but instead of just asking Eric to address them I've taken
> >> care of them and ammended the commit where necessary. There's a
> >> few issues he reported which I also address and fix in this series.
> >>
> >> Since we *do* expect users of these scripts to also use them on older
> >> kernels, I've also addressed not breaking calling the script for them,
> >> and gives us an easy way to easily extend our tests cases for future
> >> kernels as well.
> >>
> >> Before anyone considers these for stable as minor fixes, I'd recommend
> >> we also address the discrepancy on the read side of things: modify the
> >> test script to use diff against the target file instead of using the
> >> temp file.
> >>
> >> Eric Sandeen (2):
> >>   test_sysctl: add proc_do_large_bitmap() test case
> >>   sysctl: Fix proc_do_large_bitmap for large input buffers
> >>
> >> Luis Chamberlain (4):
> >>   test_sysctl: remove superfluous test_reqs()
> >>   test_sysctl: load module before testing for it
> >>   test_sysctl: ignore diff output on verify_diff_w()
> >>   test_sysctl: allow graceful use on older kernels
> >
> > Thanks for collecting and updating these!
> >
> > Acked-by: Kees Cook <keescook@chromium.org>
> >
> > Andrew, can you carry these?
>
> Ping?  This seems to have never made it off the list into anybody's
> tree.

Andrew, do you want me to send this to you again, or carry separately?

-Kees

-- 
Kees Cook

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

end of thread, other threads:[~2019-04-24 19:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 5/6] test_sysctl: add proc_do_large_bitmap() test case Luis Chamberlain
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

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.