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
We already call test_reqs(), no need to call it twice.
Signed-off-by: Luis Chamberlain <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
From: Eric Sandeen <[email protected]>
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 <[email protected]>
[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 <[email protected]>
---
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
From: Eric Sandeen <[email protected]>
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 <[email protected]>
Acked-by: Kees Cook <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
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
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 <[email protected]>
---
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
On Wed, Mar 20, 2019 at 3:28 PM Luis Chamberlain <[email protected]> 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 <[email protected]>
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
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(-)
>
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...
On 3/21/19 11:42 AM, Kees Cook wrote:
> On Wed, Mar 20, 2019 at 3:28 PM Luis Chamberlain <[email protected]> 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 <[email protected]>
>
> Andrew, can you carry these?
Ping? This seems to have never made it off the list into anybody's
tree.
Thanks,
-Eric
On Wed, Apr 24, 2019 at 10:42 AM Eric Sandeen <[email protected]> wrote:
>
> On 3/21/19 11:42 AM, Kees Cook wrote:
> > On Wed, Mar 20, 2019 at 3:28 PM Luis Chamberlain <[email protected]> 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 <[email protected]>
> >
> > 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