Hi,
strscpy_pad() patch set now with added test shenanigans.
This version adds 5 initial patches to the set and splits the single patch from v2 into two separate patches (6 and 7).
While doing the testing for strscpy_pad() it was noticed that there is duplication in how test modules are being fed to kselftest and also in the test modules themselves.
This set makes an attempt at adding a framework to kselftest for writing kernel test modules. It also adds a script for use in creating script test runners for kselftest. My macro-foo is not great, all criticism and suggestions very much appreciated. The design is based on test modules lib/test_printf.c, lib/test_bitmap.c, lib/test_xarray.c.
Shua, I'm by no means a kselftest expert, if this approach does not fit in with your general direction please say so.
Kees, I put the strscpy_pad() addition patch separate so if this goes in through Shua's tree (and if it goes in at all) its a single patch to grab if we want to start playing around with strscpy_pad().
Patch 1 fixes module unload for lib/test_printf in preparation for the rest of the series.
Patch 2 Adds a shell script that can be used to create shell script test runners.
Patch 3 Converts current shell script runners in tools/testing/selftests/lib/ to use the script introduced in patch 2.
Patch 4 Adds the test framework by way of a header file (inc. documentation)
Patch 5 Converts a couple of current test modules to make some use of the newly added test framework.
Patch 6 Adds strscpy_pad()
Patch 7 Adds test module for strscpy_pad() using the new framework and script.
If you are a testing geek and you would like to play with this; if you are already running a kernel built recently from your tree you may want to just apply the first 5 patches then you don't need to build/boot a new kernel, just config and build the lib/ test modules (test_printf etc.) and then:
sudo make TARGETS=lib kselftest
Late in the development of this I found that a bunch of boiler plate had to be added to the script to handle running tests with:
make O=/path/to/kout kselftest
The reason is that during the build we are in the output directory but the script is in the source directory. I get the feeling that a better understanding of how the kernel build process works would provide a better solution to this. The current solution is disappointing since removing duplication and boiler plate was the point of the whole exercise. I'd love a better way to solve this?
One final interesting note: there are 36 test modules in lib/ only 3 of them are run by kselftest from tools/testing/selfests/lib?
Thanks for looking at this, Tobin.
Tobin C. Harding (7): lib/test_printf: Add empty module_exit function kselftest: Add test runner creation script kselftest/lib: Use new shell runner to define tests kselftest: Add test module framework header lib: Use new kselftest header lib/string: Add strscpy_pad() function lib: Add test module for strscpy_pad
Documentation/dev-tools/kselftest.rst | 108 ++++++++++++- include/linux/string.h | 4 + lib/Kconfig.debug | 3 + lib/Makefile | 1 + lib/string.c | 47 +++++- lib/test_bitmap.c | 20 +-- lib/test_printf.c | 17 +-- lib/test_strscpy.c | 150 +++++++++++++++++++ tools/testing/selftests/kselftest_module.h | 48 ++++++ tools/testing/selftests/kselftest_module.sh | 75 ++++++++++ tools/testing/selftests/lib/Makefile | 2 +- tools/testing/selftests/lib/bitmap.sh | 25 ++-- tools/testing/selftests/lib/config | 1 + tools/testing/selftests/lib/prime_numbers.sh | 23 ++- tools/testing/selftests/lib/printf.sh | 25 ++-- tools/testing/selftests/lib/strscpy.sh | 17 +++ 16 files changed, 490 insertions(+), 76 deletions(-) create mode 100644 lib/test_strscpy.c create mode 100644 tools/testing/selftests/kselftest_module.h create mode 100755 tools/testing/selftests/kselftest_module.sh create mode 100755 tools/testing/selftests/lib/strscpy.sh
Currently the test_printf module does not have an exit function, this prevents the module from being unloaded. If we cannot unload the module we cannot run the tests a second time.
Add an empty exit function.
Signed-off-by: Tobin C. Harding tobin@kernel.org --- lib/test_printf.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/lib/test_printf.c b/lib/test_printf.c index 659b6cc0d483..601e8519319a 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -615,5 +615,11 @@ test_printf_init(void)
module_init(test_printf_init);
+static void __exit test_printf_exit(void) +{ +} + +module_exit(test_printf_exit); + MODULE_AUTHOR("Rasmus Villemoes linux@rasmusvillemoes.dk"); MODULE_LICENSE("GPL");
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
Currently the test_printf module does not have an exit function, this prevents the module from being unloaded. If we cannot unload the module we cannot run the tests a second time.
Add an empty exit function.
Signed-off-by: Tobin C. Harding tobin@kernel.org
Acked-by: Kees Cook keescook@chromium.org
-Kees
lib/test_printf.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/lib/test_printf.c b/lib/test_printf.c index 659b6cc0d483..601e8519319a 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -615,5 +615,11 @@ test_printf_init(void)
module_init(test_printf_init);
+static void __exit test_printf_exit(void) +{ +}
+module_exit(test_printf_exit);
MODULE_AUTHOR("Rasmus Villemoes linux@rasmusvillemoes.dk"); MODULE_LICENSE("GPL"); -- 2.20.1
Currently if we wish to use kselftest to run tests within a kernel module we write a small script to load/unload and do error reporting. There are a bunch of these under tools/testing/selftests/lib/ that are all identical except for the test name. We can reduce code duplication and improve maintainability if we have one version of this. However kselftest requires an executable for each test. We can move all the script logic to a central script then have each individual test script set the module name and call the main script. There is a little bit of boilerplate left in each script to handle building/running tests with the O=/path/to/out make option.
Add test runner creation script.
Signed-off-by: Tobin C. Harding tobin@kernel.org --- tools/testing/selftests/kselftest_module.sh | 75 +++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100755 tools/testing/selftests/kselftest_module.sh
diff --git a/tools/testing/selftests/kselftest_module.sh b/tools/testing/selftests/kselftest_module.sh new file mode 100755 index 000000000000..b5d446738614 --- /dev/null +++ b/tools/testing/selftests/kselftest_module.sh @@ -0,0 +1,75 @@ +#!/bin/sh +# 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 +# script that can call this script. +# + +# Individual test scrits should define these: +module="" # filename (without the .ko). +desc="" # Output prefix. + +modprobe="/sbin/modprobe" + +main() { + parse_args $@ + assert_root + assert_have_module + run_module +} + +parse_args() { + script=${0##*/} + + if [[ ! $# -eq 2 ]]; then + echo "Usage: $script <module_name> <description> [FAIL]" + exit 1 + fi + + module=$1 + desc=$2 +} + +assert_root() { + if [[ $EUID -ne 0 ]]; then + skip "please run as root" + fi +} + +assert_have_module() { + if ! $modprobe -q -n $module; then + skip "module $module is not found" + fi +} + +run_module() { + if $modprobe -q $module; then + $modprobe -q -r $module + say "ok" + else + fail "" + fi +} + +say() { + echo "$desc: $1" +} + + +fail() { + say "$1 [FAIL]" >&2 + exit 1 +} + +skip() { + say "$1 [SKIP]" >&2 + # Kselftest framework requirement - SKIP code is 4. + exit 4 +} + +# +# Main script +# +main $@
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
Currently if we wish to use kselftest to run tests within a kernel module we write a small script to load/unload and do error reporting. There are a bunch of these under tools/testing/selftests/lib/ that are all identical except for the test name. We can reduce code duplication and improve maintainability if we have one version of this. However kselftest requires an executable for each test. We can move all the script logic to a central script then have each individual test script set the module name and call the main script. There is a little bit of boilerplate left in each script to handle building/running tests with the O=/path/to/out make option.
Add test runner creation script.
Signed-off-by: Tobin C. Harding tobin@kernel.org
tools/testing/selftests/kselftest_module.sh | 75 +++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100755 tools/testing/selftests/kselftest_module.sh
diff --git a/tools/testing/selftests/kselftest_module.sh b/tools/testing/selftests/kselftest_module.sh new file mode 100755 index 000000000000..b5d446738614 --- /dev/null +++ b/tools/testing/selftests/kselftest_module.sh @@ -0,0 +1,75 @@ +#!/bin/sh +# 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 +# script that can call this script. +#
+# Individual test scrits should define these: +module="" # filename (without the .ko). +desc="" # Output prefix.
+modprobe="/sbin/modprobe"
+main() {
- parse_args $@
- assert_root
- assert_have_module
- run_module
+}
+parse_args() {
- script=${0##*/}
- if [[ ! $# -eq 2 ]]; then
echo "Usage: $script <module_name> <description> [FAIL]"exit 1- fi
- module=$1
- desc=$2
+}
+assert_root() {
- if [[ $EUID -ne 0 ]]; then
skip "please run as root"- fi
+}
+assert_have_module() {
- if ! $modprobe -q -n $module; then
skip "module $module is not found"- fi
+}
+run_module() {
- if $modprobe -q $module; then
$modprobe -q -r $module
Probably will need a way to pass arguments into the modprobe here, but otherwise looks fine.
say "ok"- else
fail ""- fi
+}
+say() {
- echo "$desc: $1"
+}
+fail() {
- say "$1 [FAIL]" >&2
- exit 1
+}
+skip() {
- say "$1 [SKIP]" >&2
- # Kselftest framework requirement - SKIP code is 4.
- exit 4
+}
+# +# Main script +#
+main $@
2.20.1
Reviewed-by: Kees Cook keescook@chromium.org
Hi,
nits/typos below:
On 4/2/19 2:27 PM, Kees Cook wrote:
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
Add test runner creation script.
Signed-off-by: Tobin C. Harding tobin@kernel.org
tools/testing/selftests/kselftest_module.sh | 75 +++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100755 tools/testing/selftests/kselftest_module.sh
diff --git a/tools/testing/selftests/kselftest_module.sh b/tools/testing/selftests/kselftest_module.sh new file mode 100755 index 000000000000..b5d446738614 --- /dev/null +++ b/tools/testing/selftests/kselftest_module.sh @@ -0,0 +1,75 @@ +#!/bin/sh +# 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
individual
+# script that can call this script. +#
+# Individual test scrits should define these:
scripts
+module="" # filename (without the .ko). +desc="" # Output prefix.
cheers.
On Tue, Apr 02, 2019 at 02:33:12PM -0700, Randy Dunlap wrote:
Hi,
nits/typos below:
Hey thanks for reviewing this Randy, first time I read it I got mixed up and thought the nits were against Kees' additions but I was wrong, thanks again.
Tobin
We just added a new script kselftest_module.sh that can be used to define kselftest tests that run tests within a kernel module. We can use it to reduce code duplication in all of the test runner scripts in tools/testing/selftests/lib/.
Use new shell runner tools/testing/selftests/kselftest_module.sh to define test runner scripts.
Signed-off-by: Tobin C. Harding tobin@kernel.org --- tools/testing/selftests/lib/bitmap.sh | 25 ++++++++++---------- tools/testing/selftests/lib/prime_numbers.sh | 23 +++++++++--------- tools/testing/selftests/lib/printf.sh | 25 ++++++++++---------- 3 files changed, 35 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh index 5a90006d1aea..ed4180ea0021 100755 --- a/tools/testing/selftests/lib/bitmap.sh +++ b/tools/testing/selftests/lib/bitmap.sh @@ -1,19 +1,18 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0
-# Kselftest framework requirement - SKIP code is 4. -ksft_skip=4 +module=test_bitmap +description="bitmap"
-# Runs bitmap infrastructure tests using test_bitmap kernel module -if ! /sbin/modprobe -q -n test_bitmap; then - echo "bitmap: module test_bitmap is not found [SKIP]" - exit $ksft_skip -fi +# +# Shouldn't need to edit anything below here. +#
-if /sbin/modprobe -q test_bitmap; then - /sbin/modprobe -q -r test_bitmap - echo "bitmap: ok" -else - echo "bitmap: [FAIL]" - exit 1 +file="kselftest_module.sh" +path="../$file" +if [[ ! $KBUILD_SRC == "" ]]; then + path="${KBUILD_SRC}/tools/testing/selftests/$file" fi + +$path $module $description + diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh index 78e7483c8d60..6f782386d897 100755 --- a/tools/testing/selftests/lib/prime_numbers.sh +++ b/tools/testing/selftests/lib/prime_numbers.sh @@ -2,18 +2,17 @@ # SPDX-License-Identifier: GPL-2.0 # Checks fast/slow prime_number generation for inconsistencies
-# Kselftest framework requirement - SKIP code is 4. -ksft_skip=4 +module=prime_numbers +description="prime_numbers"
-if ! /sbin/modprobe -q -n prime_numbers; then - echo "prime_numbers: module prime_numbers is not found [SKIP]" - exit $ksft_skip -fi +# +# Shouldn't need to edit anything below here. +#
-if /sbin/modprobe -q prime_numbers selftest=65536; then - /sbin/modprobe -q -r prime_numbers - echo "prime_numbers: ok" -else - echo "prime_numbers: [FAIL]" - exit 1 +file="kselftest_module.sh" +path="../$file" +if [[ ! $KBUILD_SRC == "" ]]; then + path="${KBUILD_SRC}/tools/testing/selftests/$file" fi + +$path $module $description diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh index 45a23e2d64ad..89717915d028 100755 --- a/tools/testing/selftests/lib/printf.sh +++ b/tools/testing/selftests/lib/printf.sh @@ -1,19 +1,18 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 -# Runs printf infrastructure using test_printf kernel module +# Tests the printf infrastructure using test_printf kernel module.
-# Kselftest framework requirement - SKIP code is 4. -ksft_skip=4 +module=test_printf +description="printf"
-if ! /sbin/modprobe -q -n test_printf; then - echo "printf: module test_printf is not found [SKIP]" - exit $ksft_skip -fi +# +# Shouldn't need to edit anything below here. +#
-if /sbin/modprobe -q test_printf; then - /sbin/modprobe -q -r test_printf - echo "printf: ok" -else - echo "printf: [FAIL]" - exit 1 +file="kselftest_module.sh" +path="../$file" +if [[ ! $KBUILD_SRC == "" ]]; then + path="${KBUILD_SRC}/tools/testing/selftests/$file" fi + +$path $module $description
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
We just added a new script kselftest_module.sh that can be used to define kselftest tests that run tests within a kernel module. We can use it to reduce code duplication in all of the test runner scripts in tools/testing/selftests/lib/.
Use new shell runner tools/testing/selftests/kselftest_module.sh to define test runner scripts.
Signed-off-by: Tobin C. Harding tobin@kernel.org
tools/testing/selftests/lib/bitmap.sh | 25 ++++++++++---------- tools/testing/selftests/lib/prime_numbers.sh | 23 +++++++++--------- tools/testing/selftests/lib/printf.sh | 25 ++++++++++---------- 3 files changed, 35 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh index 5a90006d1aea..ed4180ea0021 100755 --- a/tools/testing/selftests/lib/bitmap.sh +++ b/tools/testing/selftests/lib/bitmap.sh @@ -1,19 +1,18 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0
-# Kselftest framework requirement - SKIP code is 4. -ksft_skip=4 +module=test_bitmap +description="bitmap"
-# Runs bitmap infrastructure tests using test_bitmap kernel module -if ! /sbin/modprobe -q -n test_bitmap; then
echo "bitmap: module test_bitmap is not found [SKIP]"exit $ksft_skip-fi +# +# Shouldn't need to edit anything below here. +#
-if /sbin/modprobe -q test_bitmap; then
/sbin/modprobe -q -r test_bitmapecho "bitmap: ok"-else
echo "bitmap: [FAIL]"exit 1+file="kselftest_module.sh" +path="../$file" +if [[ ! $KBUILD_SRC == "" ]]; then
- path="${KBUILD_SRC}/tools/testing/selftests/$file"
fi
Can this just be reduced to something like:
. $(dirname $0)/../kselftest_module.sh
call_functions_here ...
+$path $module $description
diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh index 78e7483c8d60..6f782386d897 100755 --- a/tools/testing/selftests/lib/prime_numbers.sh +++ b/tools/testing/selftests/lib/prime_numbers.sh @@ -2,18 +2,17 @@ # SPDX-License-Identifier: GPL-2.0 # Checks fast/slow prime_number generation for inconsistencies
-# Kselftest framework requirement - SKIP code is 4. -ksft_skip=4 +module=prime_numbers +description="prime_numbers"
-if ! /sbin/modprobe -q -n prime_numbers; then
echo "prime_numbers: module prime_numbers is not found [SKIP]"exit $ksft_skip-fi +# +# Shouldn't need to edit anything below here. +#
-if /sbin/modprobe -q prime_numbers selftest=65536; then
/sbin/modprobe -q -r prime_numbersecho "prime_numbers: ok"-else
echo "prime_numbers: [FAIL]"exit 1+file="kselftest_module.sh" +path="../$file" +if [[ ! $KBUILD_SRC == "" ]]; then
- path="${KBUILD_SRC}/tools/testing/selftests/$file"
fi
+$path $module $description diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh index 45a23e2d64ad..89717915d028 100755 --- a/tools/testing/selftests/lib/printf.sh +++ b/tools/testing/selftests/lib/printf.sh @@ -1,19 +1,18 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 -# Runs printf infrastructure using test_printf kernel module +# Tests the printf infrastructure using test_printf kernel module.
-# Kselftest framework requirement - SKIP code is 4. -ksft_skip=4 +module=test_printf +description="printf"
-if ! /sbin/modprobe -q -n test_printf; then
echo "printf: module test_printf is not found [SKIP]"exit $ksft_skip-fi +# +# Shouldn't need to edit anything below here. +#
-if /sbin/modprobe -q test_printf; then
/sbin/modprobe -q -r test_printfecho "printf: ok"-else
echo "printf: [FAIL]"exit 1+file="kselftest_module.sh" +path="../$file" +if [[ ! $KBUILD_SRC == "" ]]; then
- path="${KBUILD_SRC}/tools/testing/selftests/$file"
fi
+$path $module $description
2.20.1
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
[...] diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh index 78e7483c8d60..6f782386d897 100755 --- a/tools/testing/selftests/lib/prime_numbers.sh +++ b/tools/testing/selftests/lib/prime_numbers.sh @@ -2,18 +2,17 @@ [...] -if /sbin/modprobe -q prime_numbers selftest=65536; then
Here it is! This conversion loses the "selftest=..." argument to modprobe.
And I think all of these files could be reduced to a single script that did something like:
. $path/kselftest_module.sh
run "strscpy" test_strscpy run "bitmap" test_bitmap run "prime numbers" prime_numbers selftest=65536
and kselftest_module.sh could define a "trap {...} EXIT" to perform the reporting of everything that got run.
On Tue, Apr 2, 2019 at 2:45 PM Kees Cook keescook@chromium.org wrote:
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
[...] diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh index 78e7483c8d60..6f782386d897 100755 --- a/tools/testing/selftests/lib/prime_numbers.sh +++ b/tools/testing/selftests/lib/prime_numbers.sh @@ -2,18 +2,17 @@ [...] -if /sbin/modprobe -q prime_numbers selftest=65536; then
Here it is! This conversion loses the "selftest=..." argument to modprobe.
And I think all of these files could be reduced to a single script that did something like:
. $path/kselftest_module.sh
run "strscpy" test_strscpy run "bitmap" test_bitmap run "prime numbers" prime_numbers selftest=65536
and kselftest_module.sh could define a "trap {...} EXIT" to perform the reporting of everything that got run.
Though I guess if we want separate scripts per module, ignore me on this part. :)
kselftest runs as a userspace process. Sometimes we need to test things from kernel space. One way of doing this is by creating a test module. Currently doing so requires developers to write a bunch of boiler plate in the module if kselftest is to be used to run the tests. This means we currently have a load of duplicate code to achieve these ends. If we have a uniform method for implementing test modules then we can reduce code duplication, ensure uniformity in the test framework, ease code maintenance, and reduce the work required to create tests. This all helps to encourage developers to write and run tests.
Add a C header file that can be included in test modules. This provides a single point for common test functions/macros. Implement a few macros that make up the start of the test framework.
Add documentation for new kselftest header and script to kselftest documentation.
Signed-off-by: Tobin C. Harding tobin@kernel.org --- Documentation/dev-tools/kselftest.rst | 108 ++++++++++++++++++++- tools/testing/selftests/kselftest_module.h | 48 +++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/kselftest_module.h
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index 7756f7a7c23b..fb7790d47147 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -14,6 +14,10 @@ in safe mode with a limited scope. In limited mode, cpu-hotplug test is run on a single cpu as opposed to all hotplug capable cpus, and memory hotplug test is run on 2% of hotplug capable memory instead of 10%.
+kselftest runs as a userspace process. Tests that can be written/run in +userspace may wish to use the `Test Harness`_. Tests that need to be +run in kernel space may wish to use a `Test Module`_. + Running the selftests (hotplug tests are run in limited mode) =============================================================
@@ -161,11 +165,111 @@ Contributing new tests (details)
e.g: tools/testing/selftests/android/config
+Test Module +=========== + +Kselftest tests the kernel from userspace. Sometimes things need +testing from within the kernel, one method of doing this is to create a +test module. We can tie the module into the kselftest framework by +using a shell script test runner. ``kselftest_module.sh`` is designed +to facilitate this process. There is also a header file provided to +assist writing kernel modules that are for use with kselftest: + +- ``tools/testing/kselftest/kselftest_module.h`` +- ``tools/testing/kselftest/kselftest_module.sh`` + +How to use +---------- + +Here we show the typical steps to create a test module and tie it into +kselftest. We use kselftests for lib/ as an example. + +1. Create the test module + +2. Create the test script that will run (load/unload) the module + e.g. ``tools/testing/selftests/lib/printf.sh`` + +3. Add line to config file e.g. ``tools/testing/selftests/lib/config`` + +4. Add test script to makefile e.g. ``tools/testing/selftests/lib/Makefile`` + +5. Verify it works: + +.. code-block:: sh + + # Assumes you have booted a fresh build of this kernel tree + cd /path/to/linux/tree + make kselftest-merge + make modules + sudo make modules_install + make TARGETS=lib kselftest + +Example Module +-------------- + +A bare bones test module might look like this: + +.. code-block:: c + + // SPDX-License-Identifier: GPL-2.0+ + + #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + + #include "../tools/testing/selftests/kselftest_module.h" + + KSTM_MODULE_GLOBALS(); + + /* + * Kernel module for testing the foobinator + */ + + static int __init test_function() + { + ... + } + + static void __init selftest(void) + { + KSTM_CHECK_ZERO(do_test_case("", 0)); + } + + KSTM_MODULE_LOADERS(test_foo); + MODULE_AUTHOR("John Developer jd@fooman.org"); + MODULE_LICENSE("GPL"); + +Example test script +------------------- + +.. code-block:: sh + + #!/bin/bash + # SPDX-License-Identifier: GPL-2.0+ + + module_name="test_foo" # Module name (without the .ko). + description="foo" # Output prefix. + + # + # 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_name $description + + Test Harness ============
-The kselftest_harness.h file contains useful helpers to build tests. The tests -from tools/testing/selftests/seccomp/seccomp_bpf.c can be used as example. +The kselftest_harness.h file contains useful helpers to build tests. The +test harness is for userspace testing, for kernel space testing see `Test +Module`_ above. + +The tests from tools/testing/selftests/seccomp/seccomp_bpf.c can be used as +example.
Example ------- diff --git a/tools/testing/selftests/kselftest_module.h b/tools/testing/selftests/kselftest_module.h new file mode 100644 index 000000000000..e8eafaf0941a --- /dev/null +++ b/tools/testing/selftests/kselftest_module.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +#ifndef __KSELFTEST_MODULE_H +#define __KSELFTEST_MODULE_H + +#include <linux/module.h> + +/* + * Test framework for writing test modules to be loaded by kselftest. + * See Documentation/dev-tools/kselftest.rst for an example test module. + */ + +#define KSTM_MODULE_GLOBALS() \ +static unsigned int total_tests __initdata; \ +static unsigned int failed_tests __initdata + +#define KSTM_CHECK_ZERO(x) do { \ + total_tests++; \ + if (x) { \ + pr_warn("TC failed at %s:%d\n", __func__, __LINE__); \ + failed_tests++; \ + } \ +} while (0) + +static inline int kstm_report(unsigned int total_tests, unsigned int failed_tests) +{ + if (failed_tests == 0) + pr_info("all %u tests passed\n", total_tests); + else + pr_warn("failed %u out of %u tests\n", failed_tests, total_tests); + + return failed_tests ? -EINVAL : 0; +} + +#define KSTM_MODULE_LOADERS(__module) \ +static int __init __module##_init(void) \ +{ \ + pr_info("loaded.\n"); \ + selftest(); \ + return kstm_report(total_tests, failed_tests); \ +} \ +static void __exit __module##_exit(void) \ +{ \ + pr_info("unloaded.\n"); \ +} \ +module_init(__module##_init); \ +module_exit(__module##_exit) + +#endif /* __KSELFTEST_MODULE_H */
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
kselftest runs as a userspace process. Sometimes we need to test things from kernel space. One way of doing this is by creating a test module. Currently doing so requires developers to write a bunch of boiler plate in the module if kselftest is to be used to run the tests. This means we currently have a load of duplicate code to achieve these ends. If we have a uniform method for implementing test modules then we can reduce code duplication, ensure uniformity in the test framework, ease code maintenance, and reduce the work required to create tests. This all helps to encourage developers to write and run tests.
Add a C header file that can be included in test modules. This provides a single point for common test functions/macros. Implement a few macros that make up the start of the test framework.
Add documentation for new kselftest header and script to kselftest documentation.
Signed-off-by: Tobin C. Harding tobin@kernel.org
I like this!
Acked-by: Kees Cook keescook@chromium.org
-Kees
Documentation/dev-tools/kselftest.rst | 108 ++++++++++++++++++++- tools/testing/selftests/kselftest_module.h | 48 +++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/kselftest_module.h
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index 7756f7a7c23b..fb7790d47147 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -14,6 +14,10 @@ in safe mode with a limited scope. In limited mode, cpu-hotplug test is run on a single cpu as opposed to all hotplug capable cpus, and memory hotplug test is run on 2% of hotplug capable memory instead of 10%.
+kselftest runs as a userspace process. Tests that can be written/run in +userspace may wish to use the `Test Harness`_. Tests that need to be +run in kernel space may wish to use a `Test Module`_.
Running the selftests (hotplug tests are run in limited mode)
@@ -161,11 +165,111 @@ Contributing new tests (details)
e.g: tools/testing/selftests/android/config+Test Module +===========
+Kselftest tests the kernel from userspace. Sometimes things need +testing from within the kernel, one method of doing this is to create a +test module. We can tie the module into the kselftest framework by +using a shell script test runner. ``kselftest_module.sh`` is designed +to facilitate this process. There is also a header file provided to +assist writing kernel modules that are for use with kselftest:
+- ``tools/testing/kselftest/kselftest_module.h`` +- ``tools/testing/kselftest/kselftest_module.sh``
+How to use +----------
+Here we show the typical steps to create a test module and tie it into +kselftest. We use kselftests for lib/ as an example.
+1. Create the test module
+2. Create the test script that will run (load/unload) the module
- e.g. ``tools/testing/selftests/lib/printf.sh``
+3. Add line to config file e.g. ``tools/testing/selftests/lib/config``
+4. Add test script to makefile e.g. ``tools/testing/selftests/lib/Makefile``
+5. Verify it works:
+.. code-block:: sh
- # Assumes you have booted a fresh build of this kernel tree
- cd /path/to/linux/tree
- make kselftest-merge
- make modules
- sudo make modules_install
- make TARGETS=lib kselftest
+Example Module +--------------
+A bare bones test module might look like this:
+.. code-block:: c
- // SPDX-License-Identifier: GPL-2.0+
- #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
- #include "../tools/testing/selftests/kselftest_module.h"
- KSTM_MODULE_GLOBALS();
- /*
- Kernel module for testing the foobinator
- */
- static int __init test_function()
- {
...- }
- static void __init selftest(void)
- {
KSTM_CHECK_ZERO(do_test_case("", 0));- }
- KSTM_MODULE_LOADERS(test_foo);
- MODULE_AUTHOR("John Developer jd@fooman.org");
- MODULE_LICENSE("GPL");
+Example test script +-------------------
+.. code-block:: sh
- #!/bin/bash
- # SPDX-License-Identifier: GPL-2.0+
- module_name="test_foo" # Module name (without the .ko).
- description="foo" # Output prefix.
- #
- # 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_name $description
Test Harness
-The kselftest_harness.h file contains useful helpers to build tests. The tests -from tools/testing/selftests/seccomp/seccomp_bpf.c can be used as example. +The kselftest_harness.h file contains useful helpers to build tests. The +test harness is for userspace testing, for kernel space testing see `Test +Module`_ above.
+The tests from tools/testing/selftests/seccomp/seccomp_bpf.c can be used as +example.
Example
diff --git a/tools/testing/selftests/kselftest_module.h b/tools/testing/selftests/kselftest_module.h new file mode 100644 index 000000000000..e8eafaf0941a --- /dev/null +++ b/tools/testing/selftests/kselftest_module.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +#ifndef __KSELFTEST_MODULE_H +#define __KSELFTEST_MODULE_H
+#include <linux/module.h>
+/*
- Test framework for writing test modules to be loaded by kselftest.
- See Documentation/dev-tools/kselftest.rst for an example test module.
- */
+#define KSTM_MODULE_GLOBALS() \ +static unsigned int total_tests __initdata; \ +static unsigned int failed_tests __initdata
+#define KSTM_CHECK_ZERO(x) do { \
total_tests++; \if (x) { \pr_warn("TC failed at %s:%d\n", __func__, __LINE__); \failed_tests++; \} \+} while (0)
+static inline int kstm_report(unsigned int total_tests, unsigned int failed_tests) +{
if (failed_tests == 0)pr_info("all %u tests passed\n", total_tests);elsepr_warn("failed %u out of %u tests\n", failed_tests, total_tests);return failed_tests ? -EINVAL : 0;+}
+#define KSTM_MODULE_LOADERS(__module) \ +static int __init __module##_init(void) \ +{ \
pr_info("loaded.\n"); \selftest(); \return kstm_report(total_tests, failed_tests); \+} \ +static void __exit __module##_exit(void) \ +{ \
pr_info("unloaded.\n"); \+} \ +module_init(__module##_init); \ +module_exit(__module##_exit)
+#endif /* __KSELFTEST_MODULE_H */
2.20.1
We just added a new C header file for use with test modules that are intended to be run with kselftest. We can reduce code duplication by using this header.
Use new kselftest header to reduce code duplication in test_printf and test_bitmap test modules.
Signed-off-by: Tobin C. Harding tobin@kernel.org --- lib/test_bitmap.c | 20 ++++---------------- lib/test_printf.c | 23 +++++------------------ 2 files changed, 9 insertions(+), 34 deletions(-)
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c index 6cd7d0740005..792d90608052 100644 --- a/lib/test_bitmap.c +++ b/lib/test_bitmap.c @@ -12,6 +12,8 @@ #include <linux/slab.h> #include <linux/string.h>
+#include "../tools/testing/selftests/kselftest_module.h" + static unsigned total_tests __initdata; static unsigned failed_tests __initdata;
@@ -361,7 +363,7 @@ static void noinline __init test_mem_optimisations(void) } }
-static int __init test_bitmap_init(void) +static void __init selftest(void) { test_zero_clear(); test_fill_set(); @@ -369,22 +371,8 @@ static int __init test_bitmap_init(void) test_bitmap_arr32(); test_bitmap_parselist(); test_mem_optimisations(); - - if (failed_tests == 0) - pr_info("all %u tests passed\n", total_tests); - else - pr_warn("failed %u out of %u tests\n", - failed_tests, total_tests); - - return failed_tests ? -EINVAL : 0; }
-static void __exit test_bitmap_cleanup(void) -{ -} - -module_init(test_bitmap_init); -module_exit(test_bitmap_cleanup); - +KSTM_MODULE_LOADERS(test_bitmap); MODULE_AUTHOR("david decotigny david.decotigny@googlers.com"); MODULE_LICENSE("GPL"); diff --git a/lib/test_printf.c b/lib/test_printf.c index 601e8519319a..f4fcc1c43739 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -21,6 +21,8 @@ #include <linux/gfp.h> #include <linux/mm.h>
+#include "../tools/testing/selftests/kselftest_module.h" + #define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -590,12 +592,11 @@ test_pointer(void) flags(); }
-static int __init -test_printf_init(void) +static void __init selftest(void) { alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL); if (!alloced_buffer) - return -ENOMEM; + return; test_buffer = alloced_buffer + PAD_SIZE;
test_basic(); @@ -604,22 +605,8 @@ test_printf_init(void) test_pointer();
kfree(alloced_buffer); - - if (failed_tests == 0) - pr_info("all %u tests passed\n", total_tests); - else - pr_warn("failed %u out of %u tests\n", failed_tests, total_tests); - - return failed_tests ? -EINVAL : 0; }
-module_init(test_printf_init); - -static void __exit test_printf_exit(void) -{ -} - -module_exit(test_printf_exit); - +KSTM_MODULE_LOADERS(test_printf); MODULE_AUTHOR("Rasmus Villemoes linux@rasmusvillemoes.dk"); MODULE_LICENSE("GPL");
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
We just added a new C header file for use with test modules that are intended to be run with kselftest. We can reduce code duplication by using this header.
Use new kselftest header to reduce code duplication in test_printf and test_bitmap test modules.
Signed-off-by: Tobin C. Harding tobin@kernel.org
Nice consolidation.
Acked-by: Kees Cook keescook@chromium.org
-Kees
lib/test_bitmap.c | 20 ++++---------------- lib/test_printf.c | 23 +++++------------------ 2 files changed, 9 insertions(+), 34 deletions(-)
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c index 6cd7d0740005..792d90608052 100644 --- a/lib/test_bitmap.c +++ b/lib/test_bitmap.c @@ -12,6 +12,8 @@ #include <linux/slab.h> #include <linux/string.h>
+#include "../tools/testing/selftests/kselftest_module.h"
static unsigned total_tests __initdata; static unsigned failed_tests __initdata;
@@ -361,7 +363,7 @@ static void noinline __init test_mem_optimisations(void) } }
-static int __init test_bitmap_init(void) +static void __init selftest(void) { test_zero_clear(); test_fill_set(); @@ -369,22 +371,8 @@ static int __init test_bitmap_init(void) test_bitmap_arr32(); test_bitmap_parselist(); test_mem_optimisations();
if (failed_tests == 0)pr_info("all %u tests passed\n", total_tests);elsepr_warn("failed %u out of %u tests\n",failed_tests, total_tests);return failed_tests ? -EINVAL : 0;}
-static void __exit test_bitmap_cleanup(void) -{ -}
-module_init(test_bitmap_init); -module_exit(test_bitmap_cleanup);
+KSTM_MODULE_LOADERS(test_bitmap); MODULE_AUTHOR("david decotigny david.decotigny@googlers.com"); MODULE_LICENSE("GPL"); diff --git a/lib/test_printf.c b/lib/test_printf.c index 601e8519319a..f4fcc1c43739 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -21,6 +21,8 @@ #include <linux/gfp.h> #include <linux/mm.h>
+#include "../tools/testing/selftests/kselftest_module.h"
#define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -590,12 +592,11 @@ test_pointer(void) flags(); }
-static int __init -test_printf_init(void) +static void __init selftest(void) { alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL); if (!alloced_buffer)
return -ENOMEM;
return; test_buffer = alloced_buffer + PAD_SIZE; test_basic();@@ -604,22 +605,8 @@ test_printf_init(void) test_pointer();
kfree(alloced_buffer);
if (failed_tests == 0)pr_info("all %u tests passed\n", total_tests);elsepr_warn("failed %u out of %u tests\n", failed_tests, total_tests);return failed_tests ? -EINVAL : 0;}
-module_init(test_printf_init);
-static void __exit test_printf_exit(void) -{ -}
-module_exit(test_printf_exit);
+KSTM_MODULE_LOADERS(test_printf); MODULE_AUTHOR("Rasmus Villemoes linux@rasmusvillemoes.dk"); MODULE_LICENSE("GPL"); -- 2.20.1
We have a function to copy strings safely and we have a function to copy strings and zero the tail of the destination (if source string is shorter than destination buffer) but we do not have a function to do both at once. This means developers must write this themselves if they desire this functionality. This is a chore, and also leaves us open to off by one errors unnecessarily.
Add a function that calls strscpy() then memset()s the tail to zero if the source string is shorter than the destination buffer.
Signed-off-by: Tobin C. Harding tobin@kernel.org --- include/linux/string.h | 4 ++++ lib/string.c | 47 +++++++++++++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h index 7927b875f80c..bfe95bf5d07e 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -31,6 +31,10 @@ size_t strlcpy(char *, const char *, size_t); #ifndef __HAVE_ARCH_STRSCPY ssize_t strscpy(char *, const char *, size_t); #endif + +/* Wraps calls to strscpy()/memset(), no arch specific code required */ +ssize_t strscpy_pad(char *dest, const char *src, size_t count); + #ifndef __HAVE_ARCH_STRCAT extern char * strcat(char *, const char *); #endif diff --git a/lib/string.c b/lib/string.c index 38e4ca08e757..3a3353512184 100644 --- a/lib/string.c +++ b/lib/string.c @@ -159,11 +159,9 @@ EXPORT_SYMBOL(strlcpy); * @src: Where to copy the string from * @count: Size of destination buffer * - * Copy the string, or as much of it as fits, into the dest buffer. - * The routine returns the number of characters copied (not including - * the trailing NUL) or -E2BIG if the destination buffer wasn't big enough. - * The behavior is undefined if the string buffers overlap. - * The destination buffer is always NUL terminated, unless it's zero-sized. + * Copy the string, or as much of it as fits, into the dest buffer. The + * behavior is undefined if the string buffers overlap. The destination + * buffer is always NUL terminated, unless it's zero-sized. * * Preferred to strlcpy() since the API doesn't require reading memory * from the src string beyond the specified "count" bytes, and since @@ -173,8 +171,10 @@ EXPORT_SYMBOL(strlcpy); * * Preferred to strncpy() since it always returns a valid string, and * doesn't unnecessarily force the tail of the destination buffer to be - * zeroed. If the zeroing is desired, it's likely cleaner to use strscpy() - * with an overflow test, then just memset() the tail of the dest buffer. + * zeroed. If zeroing is desired please use strscpy_pad(). + * + * Return: The number of characters copied (not including the trailing + * %NUL) or -E2BIG if the destination buffer wasn't big enough. */ ssize_t strscpy(char *dest, const char *src, size_t count) { @@ -237,6 +237,39 @@ ssize_t strscpy(char *dest, const char *src, size_t count) EXPORT_SYMBOL(strscpy); #endif
+/** + * strscpy_pad() - Copy a C-string into a sized buffer + * @dest: Where to copy the string to + * @src: Where to copy the string from + * @count: Size of destination buffer + * + * Copy the string, or as much of it as fits, into the dest buffer. The + * behavior is undefined if the string buffers overlap. The destination + * buffer is always %NUL terminated, unless it's zero-sized. + * + * If the source string is shorter than the destination buffer, zeros + * the tail of the destination buffer. + * + * For full explanation of why you may want to consider using the + * 'strscpy' functions please see the function docstring for strscpy(). + * + * Return: The number of characters copied (not including the trailing + * %NUL) or -E2BIG if the destination buffer wasn't big enough. + */ +ssize_t strscpy_pad(char *dest, const char *src, size_t count) +{ + ssize_t written; + + written = strscpy(dest, src, count); + if (written < 0 || written == count - 1) + return written; + + memset(dest + written + 1, 0, count - written - 1); + + return written; +} +EXPORT_SYMBOL(strscpy_pad); + #ifndef __HAVE_ARCH_STRCAT /** * strcat - Append one %NUL-terminated string to another
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
We have a function to copy strings safely and we have a function to copy strings and zero the tail of the destination (if source string is shorter than destination buffer) but we do not have a function to do both at once. This means developers must write this themselves if they desire this functionality. This is a chore, and also leaves us open to off by one errors unnecessarily.
Add a function that calls strscpy() then memset()s the tail to zero if the source string is shorter than the destination buffer.
Signed-off-by: Tobin C. Harding tobin@kernel.org
Lovely. :)
Acked-by: Kees Cook keescook@chromium.org
-Kees
include/linux/string.h | 4 ++++ lib/string.c | 47 +++++++++++++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h index 7927b875f80c..bfe95bf5d07e 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -31,6 +31,10 @@ size_t strlcpy(char *, const char *, size_t); #ifndef __HAVE_ARCH_STRSCPY ssize_t strscpy(char *, const char *, size_t); #endif
+/* Wraps calls to strscpy()/memset(), no arch specific code required */ +ssize_t strscpy_pad(char *dest, const char *src, size_t count);
#ifndef __HAVE_ARCH_STRCAT extern char * strcat(char *, const char *); #endif diff --git a/lib/string.c b/lib/string.c index 38e4ca08e757..3a3353512184 100644 --- a/lib/string.c +++ b/lib/string.c @@ -159,11 +159,9 @@ EXPORT_SYMBOL(strlcpy);
- @src: Where to copy the string from
- @count: Size of destination buffer
- Copy the string, or as much of it as fits, into the dest buffer.
- The routine returns the number of characters copied (not including
- the trailing NUL) or -E2BIG if the destination buffer wasn't big enough.
- The behavior is undefined if the string buffers overlap.
- The destination buffer is always NUL terminated, unless it's zero-sized.
- Copy the string, or as much of it as fits, into the dest buffer. The
- behavior is undefined if the string buffers overlap. The destination
- buffer is always NUL terminated, unless it's zero-sized.
- Preferred to strlcpy() since the API doesn't require reading memory
- from the src string beyond the specified "count" bytes, and since
@@ -173,8 +171,10 @@ EXPORT_SYMBOL(strlcpy);
- Preferred to strncpy() since it always returns a valid string, and
- doesn't unnecessarily force the tail of the destination buffer to be
- zeroed. If the zeroing is desired, it's likely cleaner to use strscpy()
- with an overflow test, then just memset() the tail of the dest buffer.
- zeroed. If zeroing is desired please use strscpy_pad().
- Return: The number of characters copied (not including the trailing
*/
%NUL) or -E2BIG if the destination buffer wasn't big enough.ssize_t strscpy(char *dest, const char *src, size_t count) { @@ -237,6 +237,39 @@ ssize_t strscpy(char *dest, const char *src, size_t count) EXPORT_SYMBOL(strscpy); #endif
+/**
- strscpy_pad() - Copy a C-string into a sized buffer
- @dest: Where to copy the string to
- @src: Where to copy the string from
- @count: Size of destination buffer
- Copy the string, or as much of it as fits, into the dest buffer. The
- behavior is undefined if the string buffers overlap. The destination
- buffer is always %NUL terminated, unless it's zero-sized.
- If the source string is shorter than the destination buffer, zeros
- the tail of the destination buffer.
- For full explanation of why you may want to consider using the
- 'strscpy' functions please see the function docstring for strscpy().
- Return: The number of characters copied (not including the trailing
%NUL) or -E2BIG if the destination buffer wasn't big enough.- */
+ssize_t strscpy_pad(char *dest, const char *src, size_t count) +{
ssize_t written;written = strscpy(dest, src, count);if (written < 0 || written == count - 1)return written;memset(dest + written + 1, 0, count - written - 1);return written;+} +EXPORT_SYMBOL(strscpy_pad);
#ifndef __HAVE_ARCH_STRCAT /**
- strcat - Append one %NUL-terminated string to another
-- 2.20.1
Add a test module for the new strscpy_pad() function. Tie it into the kselftest infrastructure for lib/ tests.
Signed-off-by: Tobin C. Harding tobin@kernel.org --- lib/Kconfig.debug | 3 + lib/Makefile | 1 + lib/test_strscpy.c | 150 +++++++++++++++++++++++++ tools/testing/selftests/lib/Makefile | 2 +- tools/testing/selftests/lib/config | 1 + tools/testing/selftests/lib/strscpy.sh | 17 +++ 6 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 lib/test_strscpy.c create mode 100755 tools/testing/selftests/lib/strscpy.sh
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index d4df5b24d75e..441c1571495c 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1805,6 +1805,9 @@ config TEST_HEXDUMP config TEST_STRING_HELPERS tristate "Test functions located in the string_helpers module at runtime"
+config TEST_STRSCPY + tristate "Test strscpy*() family of functions at runtime" + config TEST_KSTRTOX tristate "Test kstrto*() family of functions at runtime"
diff --git a/lib/Makefile b/lib/Makefile index e1b59da71418..82e027f73a3e 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -68,6 +68,7 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o obj-$(CONFIG_TEST_PRINTF) += test_printf.o obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o +obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o obj-$(CONFIG_TEST_BITFIELD) += test_bitfield.o obj-$(CONFIG_TEST_UUID) += test_uuid.o obj-$(CONFIG_TEST_XARRAY) += test_xarray.o diff --git a/lib/test_strscpy.c b/lib/test_strscpy.c new file mode 100644 index 000000000000..95665e8a0f97 --- /dev/null +++ b/lib/test_strscpy.c @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/string.h> + +#include "../tools/testing/selftests/kselftest_module.h" + +/* + * Kernel module for testing 'strscpy' family of functions. + */ + +KSTM_MODULE_GLOBALS(); + +/* + * tc() - Run a specific test case. + * @src: Source string, argument to strscpy_pad() + * @count: Size of destination buffer, argument to strscpy_pad() + * @expected: Expected return value from call to strscpy_pad() + * @terminator: 1 if there should be a terminating null byte 0 otherwise. + * @chars: Number of characters from the src string expected to be + * written to the dst buffer. + * @pad: Number of pad characters expected (in the tail of dst buffer). + * (@pad does not include the null terminator byte.) + * + * Calls strscpy_pad() and verifies the return value and state of the + * destination buffer after the call returns. + */ +static int __init tc(char *src, int count, int expected, + int chars, int terminator, int pad) +{ + int nr_bytes_poison; + int max_expected; + int max_count; + int written; + char buf[6]; + int index, i; + const char POISON = 'z'; + + total_tests++; + + if (!src) { + pr_err("null source string not supported\n"); + return -1; + } + + memset(buf, POISON, sizeof(buf)); + /* Future proofing test suite, validate args */ + max_count = sizeof(buf) - 2; /* Space for null and to verify overflow */ + max_expected = count - 1; /* Space for the null */ + if (count > max_count) { + pr_err("count (%d) is too big (%d) ... aborting", count, max_count); + return -1; + } + if (expected > max_expected) { + pr_warn("expected (%d) is bigger than can possibly be returned (%d)", + expected, max_expected); + } + + written = strscpy_pad(buf, src, count); + if ((written) != (expected)) { + pr_err("%d != %d (written, expected)\n", written, expected); + goto fail; + } + + if (count && written == -E2BIG) { + if (strncmp(buf, src, count - 1) != 0) { + pr_err("buffer state invalid for -E2BIG\n"); + goto fail; + } + if (buf[count - 1] != '\0') { + pr_err("too big string is not null terminated correctly\n"); + goto fail; + } + } + + for (i = 0; i < chars; i++) { + if (buf[i] != src[i]) { + pr_err("buf[i]==%c != src[i]==%c\n", buf[i], src[i]); + goto fail; + } + } + + if (terminator) { + if (buf[count - 1] != '\0') { + pr_err("string is not null terminated correctly\n"); + goto fail; + } + } + + for (i = 0; i < pad; i++) { + index = chars + terminator + i; + if (buf[index] != '\0') { + pr_err("padding missing at index: %d\n", i); + goto fail; + } + } + + nr_bytes_poison = sizeof(buf) - chars - terminator - pad; + for (i = 0; i < nr_bytes_poison; i++) { + index = sizeof(buf) - 1 - i; /* Check from the end back */ + if (buf[index] != POISON) { + pr_err("poison value missing at index: %d\n", i); + goto fail; + } + } + + return 0; +fail: + failed_tests++; + return -1; +} + +static void __init selftest(void) +{ + /* + * tc() uses a destination buffer of size 6 and needs at + * least 2 characters spare (one for null and one to check for + * overflow). This means we should only call tc() with + * strings up to a maximum of 4 characters long and 'count' + * should not exceed 4. To test with longer strings increase + * the buffer size in tc(). + */ + + /* tc(src, count, expected, chars, terminator, pad) */ + KSTM_CHECK_ZERO(tc("a", 0, -E2BIG, 0, 0, 0)); + KSTM_CHECK_ZERO(tc("", 0, -E2BIG, 0, 0, 0)); + + KSTM_CHECK_ZERO(tc("a", 1, -E2BIG, 0, 1, 0)); + KSTM_CHECK_ZERO(tc("", 1, 0, 0, 1, 0)); + + KSTM_CHECK_ZERO(tc("ab", 2, -E2BIG, 1, 1, 0)); + KSTM_CHECK_ZERO(tc("a", 2, 1, 1, 1, 0)); + KSTM_CHECK_ZERO(tc("", 2, 0, 0, 1, 1)); + + KSTM_CHECK_ZERO(tc("abc", 3, -E2BIG, 2, 1, 0)); + KSTM_CHECK_ZERO(tc("ab", 3, 2, 2, 1, 0)); + KSTM_CHECK_ZERO(tc("a", 3, 1, 1, 1, 1)); + KSTM_CHECK_ZERO(tc("", 3, 0, 0, 1, 2)); + + KSTM_CHECK_ZERO(tc("abcd", 4, -E2BIG, 3, 1, 0)); + KSTM_CHECK_ZERO(tc("abc", 4, 3, 3, 1, 0)); + KSTM_CHECK_ZERO(tc("ab", 4, 2, 2, 1, 1)); + KSTM_CHECK_ZERO(tc("a", 4, 1, 1, 1, 2)); + KSTM_CHECK_ZERO(tc("", 4, 0, 0, 1, 3)); +} + +KSTM_MODULE_LOADERS(test_strscpy); +MODULE_AUTHOR("Tobin C. Harding tobin@kernel.org"); +MODULE_LICENSE("GPL"); diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile index 70d5711e3ac8..9f26635f3e57 100644 --- a/tools/testing/selftests/lib/Makefile +++ b/tools/testing/selftests/lib/Makefile @@ -3,6 +3,6 @@ # No binaries, but make sure arg-less "make" doesn't trigger "run_tests" all:
-TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh +TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh
include ../lib.mk diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config index 126933bcc950..14a77ea4a8da 100644 --- a/tools/testing/selftests/lib/config +++ b/tools/testing/selftests/lib/config @@ -1,3 +1,4 @@ CONFIG_TEST_PRINTF=m CONFIG_TEST_BITMAP=m CONFIG_PRIME_NUMBERS=m +CONFIG_TEST_STRSCPY=m diff --git a/tools/testing/selftests/lib/strscpy.sh b/tools/testing/selftests/lib/strscpy.sh new file mode 100755 index 000000000000..f3ba4b90e602 --- /dev/null +++ b/tools/testing/selftests/lib/strscpy.sh @@ -0,0 +1,17 @@ +#!/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
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
Add a test module for the new strscpy_pad() function. Tie it into the kselftest infrastructure for lib/ tests.
Signed-off-by: Tobin C. Harding tobin@kernel.org
Yay! :)
Acked-by: Kees Cook keescook@chromium.org
-Kees
lib/Kconfig.debug | 3 + lib/Makefile | 1 + lib/test_strscpy.c | 150 +++++++++++++++++++++++++ tools/testing/selftests/lib/Makefile | 2 +- tools/testing/selftests/lib/config | 1 + tools/testing/selftests/lib/strscpy.sh | 17 +++ 6 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 lib/test_strscpy.c create mode 100755 tools/testing/selftests/lib/strscpy.sh
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index d4df5b24d75e..441c1571495c 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1805,6 +1805,9 @@ config TEST_HEXDUMP config TEST_STRING_HELPERS tristate "Test functions located in the string_helpers module at runtime"
+config TEST_STRSCPY
tristate "Test strscpy*() family of functions at runtime"config TEST_KSTRTOX tristate "Test kstrto*() family of functions at runtime"
diff --git a/lib/Makefile b/lib/Makefile index e1b59da71418..82e027f73a3e 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -68,6 +68,7 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o obj-$(CONFIG_TEST_PRINTF) += test_printf.o obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o +obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o obj-$(CONFIG_TEST_BITFIELD) += test_bitfield.o obj-$(CONFIG_TEST_UUID) += test_uuid.o obj-$(CONFIG_TEST_XARRAY) += test_xarray.o diff --git a/lib/test_strscpy.c b/lib/test_strscpy.c new file mode 100644 index 000000000000..95665e8a0f97 --- /dev/null +++ b/lib/test_strscpy.c @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: GPL-2.0+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/string.h>
+#include "../tools/testing/selftests/kselftest_module.h"
+/*
- Kernel module for testing 'strscpy' family of functions.
- */
+KSTM_MODULE_GLOBALS();
+/*
- tc() - Run a specific test case.
- @src: Source string, argument to strscpy_pad()
- @count: Size of destination buffer, argument to strscpy_pad()
- @expected: Expected return value from call to strscpy_pad()
- @terminator: 1 if there should be a terminating null byte 0 otherwise.
- @chars: Number of characters from the src string expected to be
written to the dst buffer.
- @pad: Number of pad characters expected (in the tail of dst buffer).
(@pad does not include the null terminator byte.)
- Calls strscpy_pad() and verifies the return value and state of the
- destination buffer after the call returns.
- */
+static int __init tc(char *src, int count, int expected,
int chars, int terminator, int pad)+{
int nr_bytes_poison;int max_expected;int max_count;int written;char buf[6];int index, i;const char POISON = 'z';total_tests++;if (!src) {pr_err("null source string not supported\n");return -1;}memset(buf, POISON, sizeof(buf));/* Future proofing test suite, validate args */max_count = sizeof(buf) - 2; /* Space for null and to verify overflow */max_expected = count - 1; /* Space for the null */if (count > max_count) {pr_err("count (%d) is too big (%d) ... aborting", count, max_count);return -1;}if (expected > max_expected) {pr_warn("expected (%d) is bigger than can possibly be returned (%d)",expected, max_expected);}written = strscpy_pad(buf, src, count);if ((written) != (expected)) {pr_err("%d != %d (written, expected)\n", written, expected);goto fail;}if (count && written == -E2BIG) {if (strncmp(buf, src, count - 1) != 0) {pr_err("buffer state invalid for -E2BIG\n");goto fail;}if (buf[count - 1] != '\0') {pr_err("too big string is not null terminated correctly\n");goto fail;}}for (i = 0; i < chars; i++) {if (buf[i] != src[i]) {pr_err("buf[i]==%c != src[i]==%c\n", buf[i], src[i]);goto fail;}}if (terminator) {if (buf[count - 1] != '\0') {pr_err("string is not null terminated correctly\n");goto fail;}}for (i = 0; i < pad; i++) {index = chars + terminator + i;if (buf[index] != '\0') {pr_err("padding missing at index: %d\n", i);goto fail;}}nr_bytes_poison = sizeof(buf) - chars - terminator - pad;for (i = 0; i < nr_bytes_poison; i++) {index = sizeof(buf) - 1 - i; /* Check from the end back */if (buf[index] != POISON) {pr_err("poison value missing at index: %d\n", i);goto fail;}}return 0;+fail:
failed_tests++;return -1;+}
+static void __init selftest(void) +{
/** tc() uses a destination buffer of size 6 and needs at* least 2 characters spare (one for null and one to check for* overflow). This means we should only call tc() with* strings up to a maximum of 4 characters long and 'count'* should not exceed 4. To test with longer strings increase* the buffer size in tc().*//* tc(src, count, expected, chars, terminator, pad) */KSTM_CHECK_ZERO(tc("a", 0, -E2BIG, 0, 0, 0));KSTM_CHECK_ZERO(tc("", 0, -E2BIG, 0, 0, 0));KSTM_CHECK_ZERO(tc("a", 1, -E2BIG, 0, 1, 0));KSTM_CHECK_ZERO(tc("", 1, 0, 0, 1, 0));KSTM_CHECK_ZERO(tc("ab", 2, -E2BIG, 1, 1, 0));KSTM_CHECK_ZERO(tc("a", 2, 1, 1, 1, 0));KSTM_CHECK_ZERO(tc("", 2, 0, 0, 1, 1));KSTM_CHECK_ZERO(tc("abc", 3, -E2BIG, 2, 1, 0));KSTM_CHECK_ZERO(tc("ab", 3, 2, 2, 1, 0));KSTM_CHECK_ZERO(tc("a", 3, 1, 1, 1, 1));KSTM_CHECK_ZERO(tc("", 3, 0, 0, 1, 2));KSTM_CHECK_ZERO(tc("abcd", 4, -E2BIG, 3, 1, 0));KSTM_CHECK_ZERO(tc("abc", 4, 3, 3, 1, 0));KSTM_CHECK_ZERO(tc("ab", 4, 2, 2, 1, 1));KSTM_CHECK_ZERO(tc("a", 4, 1, 1, 1, 2));KSTM_CHECK_ZERO(tc("", 4, 0, 0, 1, 3));+}
+KSTM_MODULE_LOADERS(test_strscpy); +MODULE_AUTHOR("Tobin C. Harding tobin@kernel.org"); +MODULE_LICENSE("GPL"); diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile index 70d5711e3ac8..9f26635f3e57 100644 --- a/tools/testing/selftests/lib/Makefile +++ b/tools/testing/selftests/lib/Makefile @@ -3,6 +3,6 @@ # No binaries, but make sure arg-less "make" doesn't trigger "run_tests" all:
-TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh +TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh
include ../lib.mk diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config index 126933bcc950..14a77ea4a8da 100644 --- a/tools/testing/selftests/lib/config +++ b/tools/testing/selftests/lib/config @@ -1,3 +1,4 @@ CONFIG_TEST_PRINTF=m CONFIG_TEST_BITMAP=m CONFIG_PRIME_NUMBERS=m +CONFIG_TEST_STRSCPY=m diff --git a/tools/testing/selftests/lib/strscpy.sh b/tools/testing/selftests/lib/strscpy.sh new file mode 100755 index 000000000000..f3ba4b90e602 --- /dev/null +++ b/tools/testing/selftests/lib/strscpy.sh @@ -0,0 +1,17 @@ +#!/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
2.20.1
On Thu, Mar 07, 2019 at 08:49:34AM +1100, Tobin C. Harding wrote:
On Thu, Mar 07, 2019 at 08:42:19AM +1100, Tobin C. Harding wrote:
Hi,
Man, I didn't see the merge window was open, I thought rc8 only came out on Sunday.
Sorry, please ignore this. Will re-send again later in the cycle.
Multiple people have suggested to me (offlist) that it is fine to go right ahead and send patches whenever.
Shuah, do you take patches to kselftest at any stage of the dev cycle?
Kees, same question please?
thanks, Tobin.
On Thu, Mar 7, 2019 at 1:19 PM Tobin C. Harding me@tobin.cc wrote:
On Thu, Mar 07, 2019 at 08:49:34AM +1100, Tobin C. Harding wrote:
On Thu, Mar 07, 2019 at 08:42:19AM +1100, Tobin C. Harding wrote:
Hi,
Man, I didn't see the merge window was open, I thought rc8 only came out on Sunday.
Sorry, please ignore this. Will re-send again later in the cycle.
Multiple people have suggested to me (offlist) that it is fine to go right ahead and send patches whenever.
Shuah, do you take patches to kselftest at any stage of the dev cycle?
Kees, same question please?
I don't official "close" my tree, but I'm certainly less likely to respond in a timeline manner near the merge window. ;) (And as a result, I haven't had a chance to look these over yet.)
On Thu, Mar 07, 2019 at 02:43:57PM -0800, Kees Cook wrote:
On Thu, Mar 7, 2019 at 1:19 PM Tobin C. Harding me@tobin.cc wrote:
On Thu, Mar 07, 2019 at 08:49:34AM +1100, Tobin C. Harding wrote:
On Thu, Mar 07, 2019 at 08:42:19AM +1100, Tobin C. Harding wrote:
Hi,
Man, I didn't see the merge window was open, I thought rc8 only came out on Sunday.
Sorry, please ignore this. Will re-send again later in the cycle.
Multiple people have suggested to me (offlist) that it is fine to go right ahead and send patches whenever.
Shuah, do you take patches to kselftest at any stage of the dev cycle?
Kees, same question please?
I don't official "close" my tree, but I'm certainly less likely to respond in a timeline manner near the merge window. ;) (And as a result, I haven't had a chance to look these over yet.)
Two weeks and I'm throwing eggs at your house.
Tobin
On Thu, Mar 7, 2019 at 9:24 PM Tobin C. Harding me@tobin.cc wrote:
Two weeks and I'm throwing eggs at your house.
If you can gets eggs from there to here I have a whole new international shipping business proposition for you. :)
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
This set makes an attempt at adding a framework to kselftest for writing kernel test modules. It also adds a script for use in creating script test runners for kselftest. My macro-foo is not great, all criticism and suggestions very much appreciated. The design is based on test modules lib/test_printf.c, lib/test_bitmap.c, lib/test_xarray.c.
Hi Shuah,
The bulk of this series is in the kselftests. Would you be willing to carry this series?
I think it might be able to use a v4 just to clean up the script-finding logic, but I might be entirely missing something about the best way to do it.
Thanks!
-Kees
On Tue, Apr 02, 2019 at 02:37:57PM -0700, Kees Cook wrote:
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
This set makes an attempt at adding a framework to kselftest for writing kernel test modules. It also adds a script for use in creating script test runners for kselftest. My macro-foo is not great, all criticism and suggestions very much appreciated. The design is based on test modules lib/test_printf.c, lib/test_bitmap.c, lib/test_xarray.c.
Hi Shuah,
The bulk of this series is in the kselftests. Would you be willing to carry this series?
I think it might be able to use a v4 just to clean up the script-finding logic, but I might be entirely missing something about the best way to do it.
v4 to come once I grok the bash stuff you suggested in 'PATCH v3.1'.
thanks, Tobin.
On 4/2/19 6:25 PM, Tobin C. Harding wrote:
On Tue, Apr 02, 2019 at 02:37:57PM -0700, Kees Cook wrote:
On Wed, Mar 6, 2019 at 1:43 PM Tobin C. Harding tobin@kernel.org wrote:
This set makes an attempt at adding a framework to kselftest for writing kernel test modules. It also adds a script for use in creating script test runners for kselftest. My macro-foo is not great, all criticism and suggestions very much appreciated. The design is based on test modules lib/test_printf.c, lib/test_bitmap.c, lib/test_xarray.c.
Hi Shuah,
The bulk of this series is in the kselftests. Would you be willing to carry this series?
I think it might be able to use a v4 just to clean up the script-finding logic, but I might be entirely missing something about the best way to do it.
v4 to come once I grok the bash stuff you suggested in 'PATCH v3.1'.
Yes I can take care of these. Will wait for v4
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Kees Cook -
Randy Dunlap -
shuah -
Tobin C. Harding -
Tobin C. Harding