Hi Kees, Thanks for reviewing. I will update in V1 soon. Br, Orson On 20 June 2017 at 03:01, Kees Cook <keescook@xxxxxxxxxxxx> wrote: > On Fri, Jun 16, 2017 at 2:28 AM, Orson Zhai <orson.zhai@xxxxxxxxxx> wrote: >> Sysctl test will fail in some items if the value of /proc/sys/kernel >> /sysctrl_writes_strict is 0 as the default value in kernel older than v4.5. >> >> Make this test more robust and compatible with older kernels by checking and >> update sysctrl_writes_strict value and restore it when test is done. >> >> Signed-off-by: Orson Zhai <orson.zhai@xxxxxxxxxx> >> --- >> tools/testing/selftests/sysctl/common_tests | 14 ++++++++++++++ >> tools/testing/selftests/sysctl/run_numerictests | 3 +++ >> tools/testing/selftests/sysctl/run_stringtests | 3 +++ >> 3 files changed, 20 insertions(+) >> >> diff --git a/tools/testing/selftests/sysctl/common_tests >> b/tools/testing/selftests/sysctl/common_tests >> index 17d534b1b7b4..f5c5c51d16f3 100644 >> --- a/tools/testing/selftests/sysctl/common_tests >> +++ b/tools/testing/selftests/sysctl/common_tests >> @@ -63,6 +63,20 @@ else >> echo "ok" >> fi >> >> +echo -n "Checking writes strict setting ... " >> +WRITES_STRICT="${SYSCTL}/kernel/sysctl_writes_strict" >> +if [ ! -e ${WRITES_STRICT} ]; then >> + echo "FAIL, but skip in case of old kernel" >&2 >> +else >> + val=$(cat ${WRITES_STRICT}) > > Can you give this a more meaningful name instead of "val". Perhaps "old_strict"? > >> + if [ "$val" = "1" ]; then >> + echo "ok" >> + else >> + echo "FAIL, strict value is 0 but force to 1 to continue" >&2 >> + echo "1" > ${WRITES_STRICT} >> + fi >> +fi >> + >> # Now that we've validated the sanity of "set_test" and "set_orig", >> # we can use those functions to set starting states before running >> # specific behavioral tests. >> diff --git a/tools/testing/selftests/sysctl/run_numerictests >> b/tools/testing/selftests/sysctl/run_numerictests >> index 8510f93f2d14..c0a98fd82c5c 100755 >> --- a/tools/testing/selftests/sysctl/run_numerictests >> +++ b/tools/testing/selftests/sysctl/run_numerictests >> @@ -7,4 +7,7 @@ TEST_STR=$(( $ORIG + 1 )) >> >> . ./common_tests >> >> +if [ ! -z ${val} ]; then >> + echo ${val} > ${WRITES_STRICT} >> +fi >> exit $rc >> diff --git a/tools/testing/selftests/sysctl/run_stringtests >> b/tools/testing/selftests/sysctl/run_stringtests >> index 90a9293d520c..ae98d66a9ec6 100755 >> --- a/tools/testing/selftests/sysctl/run_stringtests >> +++ b/tools/testing/selftests/sysctl/run_stringtests >> @@ -74,4 +74,7 @@ else >> echo "ok" >> fi >> >> +if [ ! -z ${val} ]; then >> + echo ${val} > ${WRITES_STRICT} >> +fi >> exit $rc > > And instead of open-coding these, can you instead add an "exit_test" > function to common_tests, which will do this and the exit $rc? > > -Kees > > -- > Kees Cook > Pixel Security -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html