[PATCH v3 0/7] lib/string: Add strscpy_pad() function

[PATCH v3 0/7] lib/string: Add strscpy_pad() function

[Tobin C. Harding]

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

-- 
2.20.1

[PATCH v3 1/7] lib/test_printf: Add empty module_exit function

[Tobin C. Harding]

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");
-- 
2.20.1

Re: [PATCH v3 1/7] lib/test_printf: Add empty module_exit function

[Kees Cook]

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
>


-- 
Kees Cook

[PATCH v3 2/7] kselftest: Add test runner creation script

[Tobin C. Harding]

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 $@
-- 
2.20.1

Re: [PATCH v3 2/7] kselftest: Add test runner creation script

[Kees Cook]

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>

-- 
Kees Cook

Re: [PATCH v3 2/7] kselftest: Add test runner creation script

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH v3 2/7] kselftest: Add test runner creation script

[Tobin C. Harding]

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

[PATCH v3 3/7] kselftest/lib: Use new shell runner to define tests

[Tobin C. Harding]

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
-- 
2.20.1

Re: [PATCH v3 3/7] kselftest/lib: Use new shell runner to define tests

[Kees Cook]

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_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

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_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
> --
> 2.20.1
>


-- 
Kees Cook

Re: [PATCH v3 3/7] kselftest/lib: Use new shell runner to define tests

[Kees Cook]

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.

-- 
Kees Cook

Re: [PATCH v3 3/7] kselftest/lib: Use new shell runner to define tests

[Kees Cook]

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. :)

-- 
Kees Cook

[PATCH v3 4/7] kselftest: Add test module framework header

[Tobin C. Harding]

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 */
-- 
2.20.1

Re: [PATCH v3 4/7] kselftest: Add test module framework header

[Kees Cook]

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);
> +       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 */
> --
> 2.20.1
>


-- 
Kees Cook

[PATCH v3 5/7] lib: Use new kselftest header

[Tobin C. Harding]

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");
-- 
2.20.1

Re: [PATCH v3 5/7] lib: Use new kselftest header

[Kees Cook]

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);
> -       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");
> --
> 2.20.1
>


-- 
Kees Cook

[PATCH v3 6/7] lib/string: Add strscpy_pad() function

[Tobin C. Harding]

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
-- 
2.20.1

Re: [PATCH v3 6/7] lib/string: Add strscpy_pad() function

[Kees Cook]

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
>


-- 
Kees Cook

[PATCH v3 7/7] lib: Add test module for strscpy_pad

[Tobin C. Harding]

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
-- 
2.20.1

Re: [PATCH v3 7/7] lib: Add test module for strscpy_pad

[Kees Cook]

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
>


-- 
Kees Cook

Re: [PATCH v3 0/7] lib/string: Add strscpy_pad() function

[Tobin C. Harding]

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.

thanks,
Tobin.

Re: [PATCH v3 0/7] lib/string: Add strscpy_pad() function

[Tobin C. Harding]

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.

Re: [PATCH v3 0/7] lib/string: Add strscpy_pad() function

[Kees Cook]

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.)

-- 
Kees Cook

Re: [PATCH v3 0/7] lib/string: Add strscpy_pad() function

[Tobin C. Harding]

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

Re: [PATCH v3 0/7] lib/string: Add strscpy_pad() function

[Kees Cook]

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. :)

-- 
Kees Cook

Re: [PATCH v3 0/7] lib/string: Add strscpy_pad() function

[Kees Cook]

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

-- 
Kees Cook

Re: [PATCH v3 0/7] lib/string: Add strscpy_pad() function

[Tobin C. Harding]

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.

Re: [PATCH v3 0/7] lib/string: Add strscpy_pad() function

[shuah]

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