linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Fwd: [PATCH] tools/testing/selftests/sysctl: Add pre-check to the value of writes_strict
       [not found] <20170616042255.22653-1-orson.zhai@spreadtrum.com>
@ 2017-06-16  9:28 ` Orson Zhai
  2017-06-16 14:49   ` Sumit Semwal
  2017-06-19 19:01   ` Kees Cook
  0 siblings, 2 replies; 5+ messages in thread
From: Orson Zhai @ 2017-06-16  9:28 UTC (permalink / raw)
  To: Shuah; +Cc: linux-kselftest, linux-kernel, Sumit Semwal, keescook

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@linaro.org>
---
 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})
+       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
--
2.12.2

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

* Re: [PATCH] tools/testing/selftests/sysctl: Add pre-check to the value of writes_strict
  2017-06-16  9:28 ` Fwd: [PATCH] tools/testing/selftests/sysctl: Add pre-check to the value of writes_strict Orson Zhai
@ 2017-06-16 14:49   ` Sumit Semwal
  2017-06-16 19:47     ` Orson Zhai
  2017-06-19 19:01   ` Kees Cook
  1 sibling, 1 reply; 5+ messages in thread
From: Sumit Semwal @ 2017-06-16 14:49 UTC (permalink / raw)
  To: Orson Zhai; +Cc: Shuah, linux-kselftest, LKML, Kees Cook

Hi Orson,

Thanks for the patch.

On 16 June 2017 at 14:58, Orson Zhai <orson.zhai@linaro.org> 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@linaro.org>

Please feel free to add my
Reviewed-by: Sumit Semwal <sumit.semwal@linaro.org>
Tested-by: Sumit Semwal <sumit.semwal@linaro.org>
 [sumits: tested LTS-4.4 with hikey (arm64) ]

> ---
>  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})
> +       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
> --
> 2.12.2


Best,
Sumit.

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

* Re: [PATCH] tools/testing/selftests/sysctl: Add pre-check to the value of writes_strict
  2017-06-16 14:49   ` Sumit Semwal
@ 2017-06-16 19:47     ` Orson Zhai
  0 siblings, 0 replies; 5+ messages in thread
From: Orson Zhai @ 2017-06-16 19:47 UTC (permalink / raw)
  To: Sumit Semwal; +Cc: Shuah, linux-kselftest, LKML, Kees Cook

On 16 June 2017 at 22:49, Sumit Semwal <sumit.semwal@linaro.org> wrote:
> Hi Orson,
>
> Thanks for the patch.
>
> On 16 June 2017 at 14:58, Orson Zhai <orson.zhai@linaro.org> 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@linaro.org>
>
> Please feel free to add my
> Reviewed-by: Sumit Semwal <sumit.semwal@linaro.org>
> Tested-by: Sumit Semwal <sumit.semwal@linaro.org>
>  [sumits: tested LTS-4.4 with hikey (arm64) ]

Sure. I will add them to my patch V2.

Thanks,
Orson

>
>> ---
>>  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})
>> +       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
>> --
>> 2.12.2
>
>
> Best,
> Sumit.

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

* Re: [PATCH] tools/testing/selftests/sysctl: Add pre-check to the value of writes_strict
  2017-06-16  9:28 ` Fwd: [PATCH] tools/testing/selftests/sysctl: Add pre-check to the value of writes_strict Orson Zhai
  2017-06-16 14:49   ` Sumit Semwal
@ 2017-06-19 19:01   ` Kees Cook
  2017-06-20  9:28     ` Orson Zhai
  1 sibling, 1 reply; 5+ messages in thread
From: Kees Cook @ 2017-06-19 19:01 UTC (permalink / raw)
  To: Orson Zhai; +Cc: Shuah Khan, linux-kselftest, LKML, Sumit Semwal

On Fri, Jun 16, 2017 at 2:28 AM, Orson Zhai <orson.zhai@linaro.org> 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@linaro.org>
> ---
>  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

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

* Re: [PATCH] tools/testing/selftests/sysctl: Add pre-check to the value of writes_strict
  2017-06-19 19:01   ` Kees Cook
@ 2017-06-20  9:28     ` Orson Zhai
  0 siblings, 0 replies; 5+ messages in thread
From: Orson Zhai @ 2017-06-20  9:28 UTC (permalink / raw)
  To: Kees Cook; +Cc: Shuah Khan, linux-kselftest, LKML, Sumit Semwal

Hi Kees,

Thanks for reviewing.

I will update in V1 soon.

Br,
Orson

On 20 June 2017 at 03:01, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Jun 16, 2017 at 2:28 AM, Orson Zhai <orson.zhai@linaro.org> 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@linaro.org>
>> ---
>>  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

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

end of thread, other threads:[~2017-06-20  9:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20170616042255.22653-1-orson.zhai@spreadtrum.com>
2017-06-16  9:28 ` Fwd: [PATCH] tools/testing/selftests/sysctl: Add pre-check to the value of writes_strict Orson Zhai
2017-06-16 14:49   ` Sumit Semwal
2017-06-16 19:47     ` Orson Zhai
2017-06-19 19:01   ` Kees Cook
2017-06-20  9:28     ` Orson Zhai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).