2019-04-02 22:27:08

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3.1] selftests: Clean up module testing shell script

This adjusts kselftest_module.sh to take an option "args" argument for
modprobe arguments, removes bash-isms (since some system's /bin/sh may
not be bash), and refactors the lib/ scripts into a shorter calling
convention.

Signed-off-by: Kees Cook <[email protected]>
---
This is my suggested clean-up for the module testing script for the
strscpy_pad() series...
---
tools/testing/selftests/kselftest_module.sh | 24 ++++++++++++--------
tools/testing/selftests/lib/bitmap.sh | 17 +-------------
tools/testing/selftests/lib/prime_numbers.sh | 17 ++------------
tools/testing/selftests/lib/printf.sh | 16 +------------
tools/testing/selftests/lib/strscpy.sh | 17 ++------------
5 files changed, 20 insertions(+), 71 deletions(-)

diff --git a/tools/testing/selftests/kselftest_module.sh b/tools/testing/selftests/kselftest_module.sh
index b5d446738614..20a897d21fde 100755
--- a/tools/testing/selftests/kselftest_module.sh
+++ b/tools/testing/selftests/kselftest_module.sh
@@ -2,19 +2,20 @@
# SPDX-License-Identifier: GPL-2.0+

#
-# Runs an individual test module. kselftest expects a separate
-# executable for each test. So test should each have an individial
+# Runs an individual test module. kselftest expects a separate
+# executable for each test. So test should each have an individial
# script that can call this script.
#

# Individual test scrits should define these:
module="" # filename (without the .ko).
desc="" # Output prefix.
+args="" # modprobe arguments

modprobe="/sbin/modprobe"

main() {
- parse_args $@
+ parse_args "$@"
assert_root
assert_have_module
run_module
@@ -23,17 +24,20 @@ main() {
parse_args() {
script=${0##*/}

- if [[ ! $# -eq 2 ]]; then
- echo "Usage: $script <module_name> <description> [FAIL]"
+ if [ $# -lt 2 ]; then
+ echo "Usage: $script <description> <module_name> [FAIL]"
exit 1
fi

- module=$1
- desc=$2
+ desc="$1"
+ shift || true
+ module="$1"
+ shift || true
+ args="$@"
}

assert_root() {
- if [[ $EUID -ne 0 ]]; then
+ if [ ! -w /dev ]; then
skip "please run as root"
fi
}
@@ -45,7 +49,7 @@ assert_have_module() {
}

run_module() {
- if $modprobe -q $module; then
+ if $modprobe -q $module $args; then
$modprobe -q -r $module
say "ok"
else
@@ -72,4 +76,4 @@ skip() {
#
# Main script
#
-main $@
+main "$@"
diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
index ed4180ea0021..5511dddc5c2d 100755
--- a/tools/testing/selftests/lib/bitmap.sh
+++ b/tools/testing/selftests/lib/bitmap.sh
@@ -1,18 +1,3 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0
-
-module=test_bitmap
-description="bitmap"
-
-#
-# Shouldn't need to edit anything below here.
-#
-
-file="kselftest_module.sh"
-path="../$file"
-if [[ ! $KBUILD_SRC == "" ]]; then
- path="${KBUILD_SRC}/tools/testing/selftests/$file"
-fi
-
-$path $module $description
-
+$(dirname $0)/../kselftest_module.sh "bitmap" test_bitmap
diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh
index 6f782386d897..2b6be0308a6d 100755
--- a/tools/testing/selftests/lib/prime_numbers.sh
+++ b/tools/testing/selftests/lib/prime_numbers.sh
@@ -1,18 +1,5 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0
# Checks fast/slow prime_number generation for inconsistencies
-
-module=prime_numbers
-description="prime_numbers"
-
-#
-# Shouldn't need to edit anything below here.
-#
-
-file="kselftest_module.sh"
-path="../$file"
-if [[ ! $KBUILD_SRC == "" ]]; then
- path="${KBUILD_SRC}/tools/testing/selftests/$file"
-fi
-
-$path $module $description
+$(dirname $0)/../kselftest_module.sh "prime numbers" prime_numbers \
+ selftest=65536
diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh
index 89717915d028..2ffa61da0296 100755
--- a/tools/testing/selftests/lib/printf.sh
+++ b/tools/testing/selftests/lib/printf.sh
@@ -1,18 +1,4 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0
# Tests the printf infrastructure using test_printf kernel module.
-
-module=test_printf
-description="printf"
-
-#
-# Shouldn't need to edit anything below here.
-#
-
-file="kselftest_module.sh"
-path="../$file"
-if [[ ! $KBUILD_SRC == "" ]]; then
- path="${KBUILD_SRC}/tools/testing/selftests/$file"
-fi
-
-$path $module $description
+$(dirname $0)/../kselftest_module.sh "printf" test_printf
diff --git a/tools/testing/selftests/lib/strscpy.sh b/tools/testing/selftests/lib/strscpy.sh
index f3ba4b90e602..1f26dac74896 100755
--- a/tools/testing/selftests/lib/strscpy.sh
+++ b/tools/testing/selftests/lib/strscpy.sh
@@ -1,17 +1,4 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0+
-
-module=test_strscpy
-description="strscpy"
-
-#
-# Shouldn't need to edit anything below here.
-#
-
-file="kselftest_module.sh"
-path="../$file"
-if [[ ! $KBUILD_SRC == "" ]]; then
- path="${KBUILD_SRC}/tools/testing/selftests/$file"
-fi
-
-$path $module $description
+# Test corner cases of the strscpy()-family of functions.
+$(dirname $0)/../kselftest_module.sh "strscpy" test_strscpy
--
2.17.1


--
Kees Cook


2019-04-02 22:32:38

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v3.1] selftests: Clean up module testing shell script

Hi,

Minor corrections below:

On 4/2/19 3:26 PM, Kees Cook wrote:
> This adjusts kselftest_module.sh to take an option "args" argument for
> modprobe arguments, removes bash-isms (since some system's /bin/sh may
> not be bash), and refactors the lib/ scripts into a shorter calling
> convention.
>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> This is my suggested clean-up for the module testing script for the
> strscpy_pad() series...
> ---
> tools/testing/selftests/kselftest_module.sh | 24 ++++++++++++--------
> tools/testing/selftests/lib/bitmap.sh | 17 +-------------
> tools/testing/selftests/lib/prime_numbers.sh | 17 ++------------
> tools/testing/selftests/lib/printf.sh | 16 +------------
> tools/testing/selftests/lib/strscpy.sh | 17 ++------------
> 5 files changed, 20 insertions(+), 71 deletions(-)
>
> diff --git a/tools/testing/selftests/kselftest_module.sh b/tools/testing/selftests/kselftest_module.sh
> index b5d446738614..20a897d21fde 100755
> --- a/tools/testing/selftests/kselftest_module.sh
> +++ b/tools/testing/selftests/kselftest_module.sh
> @@ -2,19 +2,20 @@
> # SPDX-License-Identifier: GPL-2.0+
>
> #
> -# Runs an individual test module. kselftest expects a separate
> -# executable for each test. So test should each have an individial
> +# Runs an individual test module. kselftest expects a separate
> +# executable for each test. So test should each have an individial

tests (?) individual

> # script that can call this script.
> #
>
> # Individual test scrits should define these:

scripts

> module="" # filename (without the .ko).
> desc="" # Output prefix.
> +args="" # modprobe arguments
>
> modprobe="/sbin/modprobe"
>
> main() {
> - parse_args $@
> + parse_args "$@"
> assert_root
> assert_have_module
> run_module
> @@ -23,17 +24,20 @@ main() {
> parse_args() {
> script=${0##*/}
>
> - if [[ ! $# -eq 2 ]]; then
> - echo "Usage: $script <module_name> <description> [FAIL]"
> + if [ $# -lt 2 ]; then
> + echo "Usage: $script <description> <module_name> [FAIL]"
> exit 1
> fi
>
> - module=$1
> - desc=$2
> + desc="$1"
> + shift || true
> + module="$1"
> + shift || true
> + args="$@"
> }
>
> assert_root() {
> - if [[ $EUID -ne 0 ]]; then
> + if [ ! -w /dev ]; then
> skip "please run as root"
> fi
> }
> @@ -45,7 +49,7 @@ assert_have_module() {
> }
>
> run_module() {
> - if $modprobe -q $module; then
> + if $modprobe -q $module $args; then
> $modprobe -q -r $module
> say "ok"
> else
> @@ -72,4 +76,4 @@ skip() {
> #
> # Main script
> #
> -main $@
> +main "$@"


ciao.
--
~Randy

2019-04-03 00:25:38

by Tobin C. Harding

[permalink] [raw]
Subject: Re: [PATCH v3.1] selftests: Clean up module testing shell script

On Tue, Apr 02, 2019 at 03:26:10PM -0700, Kees Cook wrote:
> This adjusts kselftest_module.sh to take an option "args" argument for
> modprobe arguments, removes bash-isms (since some system's /bin/sh may
> not be bash), and refactors the lib/ scripts into a shorter calling
> convention.
>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> This is my suggested clean-up for the module testing script for the
> strscpy_pad() series...

You are seriously at risk of being the most awesome code reviewer on
LKML. Beer is on me next time I see you.

I will roll this into a v4 for the strscpy_pad() set. Would you like a
Co-developed-by tag or just a mention in the commit log?

thanks,
Tobin.

2019-04-03 15:29:39

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3.1] selftests: Clean up module testing shell script

On Tue, Apr 2, 2019 at 5:24 PM Tobin C. Harding <[email protected]> wrote:
>
> On Tue, Apr 02, 2019 at 03:26:10PM -0700, Kees Cook wrote:
> > This adjusts kselftest_module.sh to take an option "args" argument for
> > modprobe arguments, removes bash-isms (since some system's /bin/sh may
> > not be bash), and refactors the lib/ scripts into a shorter calling
> > convention.
> >
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > This is my suggested clean-up for the module testing script for the
> > strscpy_pad() series...
>
> You are seriously at risk of being the most awesome code reviewer on
> LKML. Beer is on me next time I see you.

And slowest! ;)

> I will roll this into a v4 for the strscpy_pad() set. Would you like a
> Co-developed-by tag or just a mention in the commit log?

I'm happy either way. I really just tweaked it; maybe commit log?

--
Kees Cook