Re: [PATCH] selftests: sysctl: return Kselftest Skip code for skipped tests

[Kees Cook <keescook@chromium.org>]

On Tue, Jun 12, 2018 at 5:19 PM, Shuah Khan (Samsung OSG)
<shuah@kernel.org> wrote:
> When sysctl test is skipped because of unmet dependencies and/or
> unsupported configuration, it exits with error which is treated as
> a fail by the Kselftest framework. This leads to false negative result
> even when the test could not be run.
>
> Change it to return kselftest skip code when a test gets skipped to
> clearly report that the test could not be run.
>
> Changed return code to kselftest skip code in skip error legs that check
> requirements and module probe test error leg.
>
> Kselftest framework SKIP code is 4 and the framework prints appropriate
> messages to indicate that the test is skipped.
>
> Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>

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

> ---
>  tools/testing/selftests/sysctl/sysctl.sh | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
> index ec232c3cfcaa..584eb8ea780a 100755
> --- a/tools/testing/selftests/sysctl/sysctl.sh
> +++ b/tools/testing/selftests/sysctl/sysctl.sh
> @@ -14,6 +14,9 @@
>
>  # This performs a series tests against the proc sysctl interface.
>
> +# Kselftest framework requirement - SKIP code is 4.
> +ksft_skip=4
> +
>  TEST_NAME="sysctl"
>  TEST_DRIVER="test_${TEST_NAME}"
>  TEST_DIR=$(dirname $0)
> @@ -41,7 +44,7 @@ test_modprobe()
>                 echo "$0: $DIR not present" >&2
>                 echo "You must have the following enabled in your kernel:" >&2
>                 cat $TEST_DIR/config >&2
> -               exit 1
> +               exit $ksft_skip
>         fi
>  }
>
> @@ -98,28 +101,30 @@ test_reqs()
>         uid=$(id -u)
>         if [ $uid -ne 0 ]; then
>                 echo $msg must be run as root >&2
> -               exit 0
> +               exit $ksft_skip
>         fi
>
>         if ! which perl 2> /dev/null > /dev/null; then
>                 echo "$0: You need perl installed"
> -               exit 1
> +               exit $ksft_skip
>         fi
>         if ! which getconf 2> /dev/null > /dev/null; then
>                 echo "$0: You need getconf installed"
> -               exit 1
> +               exit $ksft_skip
>         fi
>         if ! which diff 2> /dev/null > /dev/null; then
>                 echo "$0: You need diff installed"
> -               exit 1
> +               exit $ksft_skip
>         fi
>  }
>
>  function load_req_mod()
>  {
> -       trap "test_modprobe" EXIT
> -
>         if [ ! -d $DIR ]; then
> +               if ! modprobe -q -n $TEST_DRIVER; then
> +                       echo "$0: module $TEST_DRIVER not found [SKIP]"
> +                       exit $ksft_skip
> +               fi
>                 modprobe $TEST_DRIVER
>                 if [ $? -ne 0 ]; then
>                         exit
> @@ -765,6 +770,7 @@ function parse_args()
>  test_reqs
>  allow_user_defaults
>  check_production_sysctl_writes_strict
> +test_modprobe
>  load_req_mod
>
>  trap "test_finish" EXIT
> --
> 2.17.0
>



-- 
Kees Cook
Pixel Security