linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [RESEND, PATCH v2] lib: overflow-test: add KUnit test of check_*_overflow functions
@ 2020-06-18 14:08 Vitor Massaru Iha
  2020-06-19  3:05 ` Kees Cook
  2020-06-25 13:58 ` kernel test robot
  0 siblings, 2 replies; 5+ messages in thread
From: Vitor Massaru Iha @ 2020-06-18 14:08 UTC (permalink / raw)
  To: kunit-dev
  Cc: keescook, linux, brendanhiggins, linux-kernel, linux-kselftest,
	davidgow, linux-kernel-mentees

This adds the conversion of the runtime tests of check_*_overflow functions,
from `lib/test_overflow.c`to KUnit tests.

The log similar to the one seen in dmesg running test_overflow.c can be
seen in `test.log`.

Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
Tested-by: David Gow <davidgow@google.com>
---
v2:
  * moved lib/test_overflow.c to lib/overflow-test.c; 
    * back to original license;
    * fixed style code;
    * keeps __initconst and added _refdata on overflow_test_cases variable;
    * keeps macros intact making asserts with the variable err;
    * removed duplicate test_s8_overflow();
  * fixed typos on commit message;
---
 lib/Kconfig.debug                        | 20 +++++++--
 lib/Makefile                             |  2 +-
 lib/{test_overflow.c => overflow-test.c} | 54 +++++++++---------------
 3 files changed, 38 insertions(+), 38 deletions(-)
 rename lib/{test_overflow.c => overflow-test.c} (96%)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d74ac0fd6b2d..fb8a3955e969 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2000,9 +2000,6 @@ config TEST_UUID
 config TEST_XARRAY
 	tristate "Test the XArray code at runtime"
 
-config TEST_OVERFLOW
-	tristate "Test check_*_overflow() functions at runtime"
-
 config TEST_RHASHTABLE
 	tristate "Perform selftest on resizable hash table"
 	help
@@ -2155,6 +2152,23 @@ config SYSCTL_KUNIT_TEST
 
 	  If unsure, say N.
 
+config OVERFLOW_KUNIT_TEST
+	tristate "KUnit test for overflow" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the overflow KUnit tests.
+
+	  KUnit tests run during boot and output the results to the debug log
+	  in TAP format (http://testanything.org/). Only useful for kernel devs
+	  running KUnit test harness and are not for inclusion into a production
+	  build.
+
+	  For more information on KUnit and unit tests in general please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config LIST_KUNIT_TEST
 	tristate "KUnit Test for Kernel Linked-list structures" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/lib/Makefile b/lib/Makefile
index b1c42c10073b..3b725c9f92d4 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -75,7 +75,6 @@ obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
 obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
 obj-$(CONFIG_TEST_LKM) += test_module.o
 obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
-obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
 obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
 obj-$(CONFIG_TEST_SORT) += test_sort.o
 obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
@@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
 # KUnit tests
 obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
 obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
+obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow-test.o
diff --git a/lib/test_overflow.c b/lib/overflow-test.c
similarity index 96%
rename from lib/test_overflow.c
rename to lib/overflow-test.c
index 7a4b6f6c5473..d40ef06b1ade 100644
--- a/lib/test_overflow.c
+++ b/lib/overflow-test.c
@@ -4,14 +4,11 @@
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <kunit/test.h>
 #include <linux/device.h>
 #include <linux/init.h>
-#include <linux/kernel.h>
 #include <linux/mm.h>
-#include <linux/module.h>
 #include <linux/overflow.h>
-#include <linux/slab.h>
-#include <linux/types.h>
 #include <linux/vmalloc.h>
 
 #define DEFINE_TEST_ARRAY(t)			\
@@ -270,7 +267,7 @@ DEFINE_TEST_FUNC(u64, "%llu");
 DEFINE_TEST_FUNC(s64, "%lld");
 #endif
 
-static int __init test_overflow_calculation(void)
+static void __init overflow_calculation_test(struct kunit *test)
 {
 	int err = 0;
 
@@ -285,10 +282,10 @@ static int __init test_overflow_calculation(void)
 	err |= test_s64_overflow();
 #endif
 
-	return err;
+	KUNIT_EXPECT_FALSE(test, err);
 }
 
-static int __init test_overflow_shift(void)
+static void __init overflow_shift_test(struct kunit *test)
 {
 	int err = 0;
 
@@ -479,7 +476,7 @@ static int __init test_overflow_shift(void)
 	err |= TEST_ONE_SHIFT(0, 31, s32, 0, false);
 	err |= TEST_ONE_SHIFT(0, 63, s64, 0, false);
 
-	return err;
+	KUNIT_EXPECT_FALSE(test, err);
 }
 
 /*
@@ -555,7 +552,7 @@ DEFINE_TEST_ALLOC(kvzalloc_node, kvfree,     0, 1, 1);
 DEFINE_TEST_ALLOC(devm_kmalloc,  devm_kfree, 1, 1, 0);
 DEFINE_TEST_ALLOC(devm_kzalloc,  devm_kfree, 1, 1, 0);
 
-static int __init test_overflow_allocation(void)
+static void __init overflow_allocation_test(struct kunit *test)
 {
 	const char device_name[] = "overflow-test";
 	struct device *dev;
@@ -563,10 +560,8 @@ static int __init test_overflow_allocation(void)
 
 	/* Create dummy device for devm_kmalloc()-family tests. */
 	dev = root_device_register(device_name);
-	if (IS_ERR(dev)) {
-		pr_warn("Cannot register test device\n");
-		return 1;
-	}
+	if (IS_ERR(dev))
+		kunit_warn(test, "Cannot register test device\n");
 
 	err |= test_kmalloc(NULL);
 	err |= test_kmalloc_node(NULL);
@@ -585,30 +580,21 @@ static int __init test_overflow_allocation(void)
 
 	device_unregister(dev);
 
-	return err;
+	KUNIT_EXPECT_FALSE(test, err);
 }
 
-static int __init test_module_init(void)
-{
-	int err = 0;
-
-	err |= test_overflow_calculation();
-	err |= test_overflow_shift();
-	err |= test_overflow_allocation();
-
-	if (err) {
-		pr_warn("FAIL!\n");
-		err = -EINVAL;
-	} else {
-		pr_info("all tests passed\n");
-	}
+static struct kunit_case __refdata overflow_test_cases[] = {
+	KUNIT_CASE(overflow_calculation_test),
+	KUNIT_CASE(overflow_shift_test),
+	KUNIT_CASE(overflow_allocation_test),
+	{}
+};
 
-	return err;
-}
+static struct kunit_suite overflow_test_suite = {
+	.name = "overflow",
+	.test_cases = overflow_test_cases,
+};
 
-static void __exit test_module_exit(void)
-{ }
+kunit_test_suites(&overflow_test_suite);
 
-module_init(test_module_init);
-module_exit(test_module_exit);
 MODULE_LICENSE("Dual MIT/GPL");

base-commit: 7bf200b3a4ac10b1b0376c70b8c66ed39eae7cdd
prerequisite-patch-id: e827b6b22f950b9f69620805a04e4a264cf4cc6a
-- 
2.26.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [RESEND, PATCH v2] lib: overflow-test: add KUnit test of check_*_overflow functions
  2020-06-18 14:08 [Linux-kernel-mentees] [RESEND, PATCH v2] lib: overflow-test: add KUnit test of check_*_overflow functions Vitor Massaru Iha
@ 2020-06-19  3:05 ` Kees Cook
  2020-06-19 21:57   ` Vitor Massaru Iha
  2020-06-25 23:43   ` Brendan Higgins via Linux-kernel-mentees
  2020-06-25 13:58 ` kernel test robot
  1 sibling, 2 replies; 5+ messages in thread
From: Kees Cook @ 2020-06-19  3:05 UTC (permalink / raw)
  To: Vitor Massaru Iha
  Cc: davidgow, linux, brendanhiggins, linux-kernel, linux-kselftest,
	linux-kernel-mentees, kunit-dev

On Thu, Jun 18, 2020 at 11:08:14AM -0300, Vitor Massaru Iha wrote:
> This adds the conversion of the runtime tests of check_*_overflow functions,
> from `lib/test_overflow.c`to KUnit tests.
> 
> The log similar to the one seen in dmesg running test_overflow.c can be
> seen in `test.log`.
> 
> Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> Tested-by: David Gow <davidgow@google.com>
> ---
> v2:
>   * moved lib/test_overflow.c to lib/overflow-test.c; 

I still don't want a dash in the filename, as this creates a difference
between the source name and the module name. While I still prefer
overflow_kunit.c, I can get over it and accept overflow_test.c :)

>     * back to original license;
>     * fixed style code;
>     * keeps __initconst and added _refdata on overflow_test_cases variable;
>     * keeps macros intact making asserts with the variable err;
>     * removed duplicate test_s8_overflow();
>   * fixed typos on commit message;
> ---
>  lib/Kconfig.debug                        | 20 +++++++--
>  lib/Makefile                             |  2 +-
>  lib/{test_overflow.c => overflow-test.c} | 54 +++++++++---------------
>  3 files changed, 38 insertions(+), 38 deletions(-)
>  rename lib/{test_overflow.c => overflow-test.c} (96%)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d74ac0fd6b2d..fb8a3955e969 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2000,9 +2000,6 @@ config TEST_UUID
>  config TEST_XARRAY
>  	tristate "Test the XArray code at runtime"
>  
> -config TEST_OVERFLOW
> -	tristate "Test check_*_overflow() functions at runtime"
> -
>  config TEST_RHASHTABLE
>  	tristate "Perform selftest on resizable hash table"
>  	help
> @@ -2155,6 +2152,23 @@ config SYSCTL_KUNIT_TEST
>  
>  	  If unsure, say N.
>  
> +config OVERFLOW_KUNIT_TEST
> +	tristate "KUnit test for overflow" if !KUNIT_ALL_TESTS
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This builds the overflow KUnit tests.
> +
> +	  KUnit tests run during boot and output the results to the debug log
> +	  in TAP format (http://testanything.org/). Only useful for kernel devs
> +	  running KUnit test harness and are not for inclusion into a production
> +	  build.
> +
> +	  For more information on KUnit and unit tests in general please refer
> +	  to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> +	  If unsure, say N.
> +
>  config LIST_KUNIT_TEST
>  	tristate "KUnit Test for Kernel Linked-list structures" if !KUNIT_ALL_TESTS
>  	depends on KUNIT
> diff --git a/lib/Makefile b/lib/Makefile
> index b1c42c10073b..3b725c9f92d4 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -75,7 +75,6 @@ obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
>  obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
>  obj-$(CONFIG_TEST_LKM) += test_module.o
>  obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
> -obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
>  obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
>  obj-$(CONFIG_TEST_SORT) += test_sort.o
>  obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
> @@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
>  # KUnit tests
>  obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
>  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> +obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow-test.o
> diff --git a/lib/test_overflow.c b/lib/overflow-test.c
> similarity index 96%
> rename from lib/test_overflow.c
> rename to lib/overflow-test.c
> index 7a4b6f6c5473..d40ef06b1ade 100644
> --- a/lib/test_overflow.c
> +++ b/lib/overflow-test.c
> @@ -4,14 +4,11 @@
>   */
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <kunit/test.h>
>  #include <linux/device.h>
>  #include <linux/init.h>
> -#include <linux/kernel.h>
>  #include <linux/mm.h>
> -#include <linux/module.h>
>  #include <linux/overflow.h>
> -#include <linux/slab.h>
> -#include <linux/types.h>
>  #include <linux/vmalloc.h>
>  
>  #define DEFINE_TEST_ARRAY(t)			\
> @@ -270,7 +267,7 @@ DEFINE_TEST_FUNC(u64, "%llu");
>  DEFINE_TEST_FUNC(s64, "%lld");
>  #endif
>  
> -static int __init test_overflow_calculation(void)
> +static void __init overflow_calculation_test(struct kunit *test)
>  {
>  	int err = 0;
>  
> @@ -285,10 +282,10 @@ static int __init test_overflow_calculation(void)
>  	err |= test_s64_overflow();
>  #endif
>  
> -	return err;
> +	KUNIT_EXPECT_FALSE(test, err);
>  }

Ah! Well, yes, I guess that is one way to do it. :) I'm just curious:
why the change away from doing EXPECTs on individual tests?

>  
> -static int __init test_overflow_shift(void)
> +static void __init overflow_shift_test(struct kunit *test)
>  {
>  	int err = 0;
>  
> @@ -479,7 +476,7 @@ static int __init test_overflow_shift(void)
>  	err |= TEST_ONE_SHIFT(0, 31, s32, 0, false);
>  	err |= TEST_ONE_SHIFT(0, 63, s64, 0, false);
>  
> -	return err;
> +	KUNIT_EXPECT_FALSE(test, err);
>  }
>  
>  /*
> @@ -555,7 +552,7 @@ DEFINE_TEST_ALLOC(kvzalloc_node, kvfree,     0, 1, 1);
>  DEFINE_TEST_ALLOC(devm_kmalloc,  devm_kfree, 1, 1, 0);
>  DEFINE_TEST_ALLOC(devm_kzalloc,  devm_kfree, 1, 1, 0);
>  
> -static int __init test_overflow_allocation(void)
> +static void __init overflow_allocation_test(struct kunit *test)
>  {
>  	const char device_name[] = "overflow-test";
>  	struct device *dev;
> @@ -563,10 +560,8 @@ static int __init test_overflow_allocation(void)
>  
>  	/* Create dummy device for devm_kmalloc()-family tests. */
>  	dev = root_device_register(device_name);
> -	if (IS_ERR(dev)) {
> -		pr_warn("Cannot register test device\n");
> -		return 1;
> -	}
> +	if (IS_ERR(dev))
> +		kunit_warn(test, "Cannot register test device\n");
>  
>  	err |= test_kmalloc(NULL);
>  	err |= test_kmalloc_node(NULL);
> @@ -585,30 +580,21 @@ static int __init test_overflow_allocation(void)
>  
>  	device_unregister(dev);
>  
> -	return err;
> +	KUNIT_EXPECT_FALSE(test, err);
>  }
>  
> -static int __init test_module_init(void)
> -{
> -	int err = 0;
> -
> -	err |= test_overflow_calculation();
> -	err |= test_overflow_shift();
> -	err |= test_overflow_allocation();
> -
> -	if (err) {
> -		pr_warn("FAIL!\n");
> -		err = -EINVAL;
> -	} else {
> -		pr_info("all tests passed\n");
> -	}
> +static struct kunit_case __refdata overflow_test_cases[] = {

Erm, __refdata? This seems like it should be __initdata.

> +	KUNIT_CASE(overflow_calculation_test),
> +	KUNIT_CASE(overflow_shift_test),
> +	KUNIT_CASE(overflow_allocation_test),
> +	{}
> +};
>  
> -	return err;
> -}
> +static struct kunit_suite overflow_test_suite = {

And this.

> +	.name = "overflow",
> +	.test_cases = overflow_test_cases,
> +};
>  
> -static void __exit test_module_exit(void)
> -{ }
> +kunit_test_suites(&overflow_test_suite);

I suspect the problem causing the need for __refdata there is the lack
of __init markings on the functions in kunit_test_suites()?

(Or maybe this is explained somewhere else I've missed it.)

For example, would this work? (I haven't tested it all.)


diff --git a/include/kunit/test.h b/include/kunit/test.h
index 59f3144f009a..aad746d59d2f 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -233,9 +233,9 @@ size_t kunit_suite_num_test_cases(struct kunit_suite *suite);
 unsigned int kunit_test_case_num(struct kunit_suite *suite,
 				 struct kunit_case *test_case);
 
-int __kunit_test_suites_init(struct kunit_suite **suites);
+int __init __kunit_test_suites_init(struct kunit_suite **suites);
 
-void __kunit_test_suites_exit(struct kunit_suite **suites);
+void __exit __kunit_test_suites_exit(struct kunit_suite **suites);
 
 /**
  * kunit_test_suites() - used to register one or more &struct kunit_suite
@@ -263,8 +263,9 @@ void __kunit_test_suites_exit(struct kunit_suite **suites);
  * everything else is definitely initialized.
  */
 #define kunit_test_suites(suites_list...)				\
-	static struct kunit_suite *suites[] = {suites_list, NULL};	\
-	static int kunit_test_suites_init(void)				\
+	static struct kunit_suite *suites[] __initdata =		\
+		{suites_list, NULL};					\
+	static int __init kunit_test_suites_init(void)			\
 	{								\
 		return __kunit_test_suites_init(suites);		\
 	}								\
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index c36037200310..bfb0f563721b 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -381,7 +381,7 @@ static void kunit_init_suite(struct kunit_suite *suite)
 	kunit_debugfs_create_suite(suite);
 }
 
-int __kunit_test_suites_init(struct kunit_suite **suites)
+int __init __kunit_test_suites_init(struct kunit_suite **suites)
 {
 	unsigned int i;
 
@@ -393,7 +393,7 @@ int __kunit_test_suites_init(struct kunit_suite **suites)
 }
 EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
 
-static void kunit_exit_suite(struct kunit_suite *suite)
+static void __exit kunit_exit_suite(struct kunit_suite *suite)
 {
 	kunit_debugfs_destroy_suite(suite);
 }

>  
> -module_init(test_module_init);
> -module_exit(test_module_exit);
>  MODULE_LICENSE("Dual MIT/GPL");
> 
> base-commit: 7bf200b3a4ac10b1b0376c70b8c66ed39eae7cdd
> prerequisite-patch-id: e827b6b22f950b9f69620805a04e4a264cf4cc6a
> -- 
> 2.26.2
> 

Thanks again for the conversion!

-- 
Kees Cook
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [RESEND, PATCH v2] lib: overflow-test: add KUnit test of check_*_overflow functions
  2020-06-19  3:05 ` Kees Cook
@ 2020-06-19 21:57   ` Vitor Massaru Iha
  2020-06-25 23:43   ` Brendan Higgins via Linux-kernel-mentees
  1 sibling, 0 replies; 5+ messages in thread
From: Vitor Massaru Iha @ 2020-06-19 21:57 UTC (permalink / raw)
  To: Kees Cook
  Cc: davidgow, linux, brendanhiggins, linux-kernel, linux-kselftest,
	linux-kernel-mentees, kunit-dev

On Thu, 2020-06-18 at 20:05 -0700, Kees Cook wrote:
> On Thu, Jun 18, 2020 at 11:08:14AM -0300, Vitor Massaru Iha wrote:
> > This adds the conversion of the runtime tests of check_*_overflow
> > functions,
> > from `lib/test_overflow.c`to KUnit tests.
> > 
> > The log similar to the one seen in dmesg running test_overflow.c
> > can be
> > seen in `test.log`.
> > 
> > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > Tested-by: David Gow <davidgow@google.com>
> > ---
> > v2:
> >   * moved lib/test_overflow.c to lib/overflow-test.c; 

Sure.

> I still don't want a dash in the filename, as this creates a
> difference
> between the source name and the module name. While I still prefer
> overflow_kunit.c, I can get over it and accept overflow_test.c :)
> 
> >     * back to original license;
> >     * fixed style code;
> >     * keeps __initconst and added _refdata on overflow_test_cases
> > variable;
> >     * keeps macros intact making asserts with the variable err;
> >     * removed duplicate test_s8_overflow();
> >   * fixed typos on commit message;
> > ---
> >  lib/Kconfig.debug                        | 20 +++++++--
> >  lib/Makefile                             |  2 +-
> >  lib/{test_overflow.c => overflow-test.c} | 54 +++++++++-----------
> > ----
> >  3 files changed, 38 insertions(+), 38 deletions(-)
> >  rename lib/{test_overflow.c => overflow-test.c} (96%)
> > 
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index d74ac0fd6b2d..fb8a3955e969 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2000,9 +2000,6 @@ config TEST_UUID
> >  config TEST_XARRAY
> >  	tristate "Test the XArray code at runtime"
> >  
> > -config TEST_OVERFLOW
> > -	tristate "Test check_*_overflow() functions at runtime"
> > -
> >  config TEST_RHASHTABLE
> >  	tristate "Perform selftest on resizable hash table"
> >  	help
> > @@ -2155,6 +2152,23 @@ config SYSCTL_KUNIT_TEST
> >  
> >  	  If unsure, say N.
> >  
> > +config OVERFLOW_KUNIT_TEST
> > +	tristate "KUnit test for overflow" if !KUNIT_ALL_TESTS
> > +	depends on KUNIT
> > +	default KUNIT_ALL_TESTS
> > +	help
> > +	  This builds the overflow KUnit tests.
> > +
> > +	  KUnit tests run during boot and output the results to the
> > debug log
> > +	  in TAP format (http://testanything.org/). Only useful for
> > kernel devs
> > +	  running KUnit test harness and are not for inclusion into a
> > production
> > +	  build.
> > +
> > +	  For more information on KUnit and unit tests in general
> > please refer
> > +	  to the KUnit documentation in Documentation/dev-tools/kunit/.
> > +
> > +	  If unsure, say N.
> > +
> >  config LIST_KUNIT_TEST
> >  	tristate "KUnit Test for Kernel Linked-list structures" if
> > !KUNIT_ALL_TESTS
> >  	depends on KUNIT
> > diff --git a/lib/Makefile b/lib/Makefile
> > index b1c42c10073b..3b725c9f92d4 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -75,7 +75,6 @@ obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
> >  obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
> >  obj-$(CONFIG_TEST_LKM) += test_module.o
> >  obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
> > -obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
> >  obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
> >  obj-$(CONFIG_TEST_SORT) += test_sort.o
> >  obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
> > @@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
> >  # KUnit tests
> >  obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> >  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> > +obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow-test.o
> > diff --git a/lib/test_overflow.c b/lib/overflow-test.c
> > similarity index 96%
> > rename from lib/test_overflow.c
> > rename to lib/overflow-test.c
> > index 7a4b6f6c5473..d40ef06b1ade 100644
> > --- a/lib/test_overflow.c
> > +++ b/lib/overflow-test.c
> > @@ -4,14 +4,11 @@
> >   */
> >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >  
> > +#include <kunit/test.h>
> >  #include <linux/device.h>
> >  #include <linux/init.h>
> > -#include <linux/kernel.h>
> >  #include <linux/mm.h>
> > -#include <linux/module.h>
> >  #include <linux/overflow.h>
> > -#include <linux/slab.h>
> > -#include <linux/types.h>
> >  #include <linux/vmalloc.h>
> >  
> >  #define DEFINE_TEST_ARRAY(t)			\
> > @@ -270,7 +267,7 @@ DEFINE_TEST_FUNC(u64, "%llu");
> >  DEFINE_TEST_FUNC(s64, "%lld");
> >  #endif
> >  
> > -static int __init test_overflow_calculation(void)
> > +static void __init overflow_calculation_test(struct kunit *test)
> >  {
> >  	int err = 0;
> >  
> > @@ -285,10 +282,10 @@ static int __init
> > test_overflow_calculation(void)
> >  	err |= test_s64_overflow();
> >  #endif
> >  
> > -	return err;
> > +	KUNIT_EXPECT_FALSE(test, err);
> >  }
> 
> Ah! Well, yes, I guess that is one way to do it. :) I'm just curious:
> why the change away from doing EXPECTs on individual tests?

When returning the err variables, I was not sure if it was to make the
asserts individually, I can do that.

> >  
> > -static int __init test_overflow_shift(void)
> > +static void __init overflow_shift_test(struct kunit *test)
> >  {
> >  	int err = 0;
> >  
> > @@ -479,7 +476,7 @@ static int __init test_overflow_shift(void)
> >  	err |= TEST_ONE_SHIFT(0, 31, s32, 0, false);
> >  	err |= TEST_ONE_SHIFT(0, 63, s64, 0, false);
> >  
> > -	return err;
> > +	KUNIT_EXPECT_FALSE(test, err);
> >  }
> >  
> >  /*
> > @@ -555,7 +552,7 @@ DEFINE_TEST_ALLOC(kvzalloc_node, kvfree,     0,
> > 1, 1);
> >  DEFINE_TEST_ALLOC(devm_kmalloc,  devm_kfree, 1, 1, 0);
> >  DEFINE_TEST_ALLOC(devm_kzalloc,  devm_kfree, 1, 1, 0);
> >  
> > -static int __init test_overflow_allocation(void)
> > +static void __init overflow_allocation_test(struct kunit *test)
> >  {
> >  	const char device_name[] = "overflow-test";
> >  	struct device *dev;
> > @@ -563,10 +560,8 @@ static int __init
> > test_overflow_allocation(void)
> >  
> >  	/* Create dummy device for devm_kmalloc()-family tests. */
> >  	dev = root_device_register(device_name);
> > -	if (IS_ERR(dev)) {
> > -		pr_warn("Cannot register test device\n");
> > -		return 1;
> > -	}
> > +	if (IS_ERR(dev))
> > +		kunit_warn(test, "Cannot register test device\n");
> >  
> >  	err |= test_kmalloc(NULL);
> >  	err |= test_kmalloc_node(NULL);
> > @@ -585,30 +580,21 @@ static int __init
> > test_overflow_allocation(void)
> >  
> >  	device_unregister(dev);
> >  
> > -	return err;
> > +	KUNIT_EXPECT_FALSE(test, err);
> >  }
> >  
> > -static int __init test_module_init(void)
> > -{
> > -	int err = 0;
> > -
> > -	err |= test_overflow_calculation();
> > -	err |= test_overflow_shift();
> > -	err |= test_overflow_allocation();
> > -
> > -	if (err) {
> > -		pr_warn("FAIL!\n");
> > -		err = -EINVAL;
> > -	} else {
> > -		pr_info("all tests passed\n");
> > -	}
> > +static struct kunit_case __refdata overflow_test_cases[] = {
> 
> Erm, __refdata? This seems like it should be __initdata.

I tried to use __initdata, but the build still gave warnings.

> 
> > +	KUNIT_CASE(overflow_calculation_test),
> > +	KUNIT_CASE(overflow_shift_test),
> > +	KUNIT_CASE(overflow_allocation_test),
> > +	{}
> > +};
> >  
> > -	return err;
> > -}
> > +static struct kunit_suite overflow_test_suite = {
> 
> And this.
> 
> > +	.name = "overflow",
> > +	.test_cases = overflow_test_cases,
> > +};
> >  
> > -static void __exit test_module_exit(void)
> > -{ }
> > +kunit_test_suites(&overflow_test_suite);
> 
> I suspect the problem causing the need for __refdata there is the
> lack
> of __init markings on the functions in kunit_test_suites()?

From the kunit_test_suites() documentation I saw that I need to write
the test as a module to solve this problem. I'll fix this.

> 
> (Or maybe this is explained somewhere else I've missed it.)
> 
> For example, would this work? (I haven't tested it all.)

Oops. It doesn't work, I'm sorry.


> 
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 59f3144f009a..aad746d59d2f 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -233,9 +233,9 @@ size_t kunit_suite_num_test_cases(struct
> kunit_suite *suite);
>  unsigned int kunit_test_case_num(struct kunit_suite *suite,
>  				 struct kunit_case *test_case);
>  
> -int __kunit_test_suites_init(struct kunit_suite **suites);
> +int __init __kunit_test_suites_init(struct kunit_suite **suites);
>  
> -void __kunit_test_suites_exit(struct kunit_suite **suites);
> +void __exit __kunit_test_suites_exit(struct kunit_suite **suites);
>  
>  /**
>   * kunit_test_suites() - used to register one or more &struct
> kunit_suite
> @@ -263,8 +263,9 @@ void __kunit_test_suites_exit(struct kunit_suite
> **suites);
>   * everything else is definitely initialized.
>   */
>  #define kunit_test_suites(suites_list...)				
> \
> -	static struct kunit_suite *suites[] = {suites_list, NULL};	
> \
> -	static int kunit_test_suites_init(void)				
> \
> +	static struct kunit_suite *suites[] __initdata =		\
> +		{suites_list, NULL};					
> \
> +	static int __init kunit_test_suites_init(void)			
> \
>  	{								\
>  		return __kunit_test_suites_init(suites);		\
>  	}								\
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index c36037200310..bfb0f563721b 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -381,7 +381,7 @@ static void kunit_init_suite(struct kunit_suite
> *suite)
>  	kunit_debugfs_create_suite(suite);
>  }
>  
> -int __kunit_test_suites_init(struct kunit_suite **suites)
> +int __init __kunit_test_suites_init(struct kunit_suite **suites)
>  {
>  	unsigned int i;
>  
> @@ -393,7 +393,7 @@ int __kunit_test_suites_init(struct kunit_suite
> **suites)
>  }
>  EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
>  
> -static void kunit_exit_suite(struct kunit_suite *suite)
> +static void __exit kunit_exit_suite(struct kunit_suite *suite)
>  {
>  	kunit_debugfs_destroy_suite(suite);
>  }
> 
> >  
> > -module_init(test_module_init);
> > -module_exit(test_module_exit);
> >  MODULE_LICENSE("Dual MIT/GPL");
> > 
> > base-commit: 7bf200b3a4ac10b1b0376c70b8c66ed39eae7cdd
> > prerequisite-patch-id: e827b6b22f950b9f69620805a04e4a264cf4cc6a
> > -- 
> > 2.26.2
> > 
> 
> Thanks again for the conversion!


Thanks for the review.

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [RESEND, PATCH v2] lib: overflow-test: add KUnit test of check_*_overflow functions
  2020-06-18 14:08 [Linux-kernel-mentees] [RESEND, PATCH v2] lib: overflow-test: add KUnit test of check_*_overflow functions Vitor Massaru Iha
  2020-06-19  3:05 ` Kees Cook
@ 2020-06-25 13:58 ` kernel test robot
  1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-06-25 13:58 UTC (permalink / raw)
  To: Vitor Massaru Iha, kunit-dev
  Cc: kbuild-all, keescook, linux, brendanhiggins, linux-kernel,
	linux-kselftest, davidgow, linux-kernel-mentees

Hi Vitor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.8-rc2]
[cannot apply to next-20200625]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vitor-Massaru-Iha/lib-overflow-test-add-KUnit-test-of-check_-_overflow-functions/20200618-221026
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1b5044021070efa3259f3e9548dc35d1eb6aa844
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


cppcheck warnings: (new ones prefixed by >>)

>> lib/overflow-test.c:334:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(1, 0, unsigned int, 1U << 0, false);
           ^
   lib/overflow-test.c:335:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(1, 20, unsigned int, 1U << 20, false);
           ^
   lib/overflow-test.c:336:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(1, 31, unsigned int, 1U << 31, false);
           ^
   lib/overflow-test.c:337:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(0xFFFFU, 16, unsigned int, 0xFFFFU << 16, false);
           ^
   lib/overflow-test.c:351:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(0, 31, unsigned int, 0, false);
           ^
   lib/overflow-test.c:365:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(1, 32, unsigned int, 0, true);
           ^
   lib/overflow-test.c:383:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(2215151766U, 1, unsigned int, 0, true);
           ^
   lib/overflow-test.c:415:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(0x100000000ULL, 0, unsigned int, 0, true);
           ^
   lib/overflow-test.c:426:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(-10, 0, unsigned int, 0, true);
           ^
   lib/overflow-test.c:438:9: warning: Checking if unsigned variable '(unsigned int)-1' is less than zero. [unsignedLessThanZero]
    err |= TEST_ONE_SHIFT(0, -15, unsigned int, 0, true);
           ^

vim +334 lib/overflow-test.c

d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  318  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  319  	/* Sane shifts. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  320  	err |= TEST_ONE_SHIFT(1, 0, u8, 1 << 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  321  	err |= TEST_ONE_SHIFT(1, 4, u8, 1 << 4, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  322  	err |= TEST_ONE_SHIFT(1, 7, u8, 1 << 7, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  323  	err |= TEST_ONE_SHIFT(0xF, 4, u8, 0xF << 4, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  324  	err |= TEST_ONE_SHIFT(1, 0, u16, 1 << 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  325  	err |= TEST_ONE_SHIFT(1, 10, u16, 1 << 10, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  326  	err |= TEST_ONE_SHIFT(1, 15, u16, 1 << 15, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  327  	err |= TEST_ONE_SHIFT(0xFF, 8, u16, 0xFF << 8, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  328  	err |= TEST_ONE_SHIFT(1, 0, int, 1 << 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  329  	err |= TEST_ONE_SHIFT(1, 16, int, 1 << 16, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  330  	err |= TEST_ONE_SHIFT(1, 30, int, 1 << 30, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  331  	err |= TEST_ONE_SHIFT(1, 0, s32, 1 << 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  332  	err |= TEST_ONE_SHIFT(1, 16, s32, 1 << 16, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  333  	err |= TEST_ONE_SHIFT(1, 30, s32, 1 << 30, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01 @334  	err |= TEST_ONE_SHIFT(1, 0, unsigned int, 1U << 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  335  	err |= TEST_ONE_SHIFT(1, 20, unsigned int, 1U << 20, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  336  	err |= TEST_ONE_SHIFT(1, 31, unsigned int, 1U << 31, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  337  	err |= TEST_ONE_SHIFT(0xFFFFU, 16, unsigned int, 0xFFFFU << 16, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  338  	err |= TEST_ONE_SHIFT(1, 0, u32, 1U << 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  339  	err |= TEST_ONE_SHIFT(1, 20, u32, 1U << 20, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  340  	err |= TEST_ONE_SHIFT(1, 31, u32, 1U << 31, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  341  	err |= TEST_ONE_SHIFT(0xFFFFU, 16, u32, 0xFFFFU << 16, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  342  	err |= TEST_ONE_SHIFT(1, 0, u64, 1ULL << 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  343  	err |= TEST_ONE_SHIFT(1, 40, u64, 1ULL << 40, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  344  	err |= TEST_ONE_SHIFT(1, 63, u64, 1ULL << 63, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  345  	err |= TEST_ONE_SHIFT(0xFFFFFFFFULL, 32, u64,
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  346  			      0xFFFFFFFFULL << 32, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  347  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  348  	/* Sane shift: start and end with 0, without a too-wide shift. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  349  	err |= TEST_ONE_SHIFT(0, 7, u8, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  350  	err |= TEST_ONE_SHIFT(0, 15, u16, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  351  	err |= TEST_ONE_SHIFT(0, 31, unsigned int, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  352  	err |= TEST_ONE_SHIFT(0, 31, u32, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  353  	err |= TEST_ONE_SHIFT(0, 63, u64, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  354  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  355  	/* Sane shift: start and end with 0, without reaching signed bit. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  356  	err |= TEST_ONE_SHIFT(0, 6, s8, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  357  	err |= TEST_ONE_SHIFT(0, 14, s16, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  358  	err |= TEST_ONE_SHIFT(0, 30, int, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  359  	err |= TEST_ONE_SHIFT(0, 30, s32, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  360  	err |= TEST_ONE_SHIFT(0, 62, s64, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  361  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  362  	/* Overflow: shifted the bit off the end. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  363  	err |= TEST_ONE_SHIFT(1, 8, u8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  364  	err |= TEST_ONE_SHIFT(1, 16, u16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  365  	err |= TEST_ONE_SHIFT(1, 32, unsigned int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  366  	err |= TEST_ONE_SHIFT(1, 32, u32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  367  	err |= TEST_ONE_SHIFT(1, 64, u64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  368  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  369  	/* Overflow: shifted into the signed bit. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  370  	err |= TEST_ONE_SHIFT(1, 7, s8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  371  	err |= TEST_ONE_SHIFT(1, 15, s16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  372  	err |= TEST_ONE_SHIFT(1, 31, int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  373  	err |= TEST_ONE_SHIFT(1, 31, s32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  374  	err |= TEST_ONE_SHIFT(1, 63, s64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  375  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  376  	/* Overflow: high bit falls off unsigned types. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  377  	/* 10010110 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  378  	err |= TEST_ONE_SHIFT(150, 1, u8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  379  	/* 1000100010010110 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  380  	err |= TEST_ONE_SHIFT(34966, 1, u16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  381  	/* 10000100000010001000100010010110 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  382  	err |= TEST_ONE_SHIFT(2215151766U, 1, u32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  383  	err |= TEST_ONE_SHIFT(2215151766U, 1, unsigned int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  384  	/* 1000001000010000010000000100000010000100000010001000100010010110 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  385  	err |= TEST_ONE_SHIFT(9372061470395238550ULL, 1, u64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  386  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  387  	/* Overflow: bit shifted into signed bit on signed types. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  388  	/* 01001011 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  389  	err |= TEST_ONE_SHIFT(75, 1, s8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  390  	/* 0100010001001011 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  391  	err |= TEST_ONE_SHIFT(17483, 1, s16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  392  	/* 01000010000001000100010001001011 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  393  	err |= TEST_ONE_SHIFT(1107575883, 1, s32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  394  	err |= TEST_ONE_SHIFT(1107575883, 1, int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  395  	/* 0100000100001000001000000010000001000010000001000100010001001011 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  396  	err |= TEST_ONE_SHIFT(4686030735197619275LL, 1, s64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  397  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  398  	/* Overflow: bit shifted past signed bit on signed types. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  399  	/* 01001011 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  400  	err |= TEST_ONE_SHIFT(75, 2, s8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  401  	/* 0100010001001011 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  402  	err |= TEST_ONE_SHIFT(17483, 2, s16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  403  	/* 01000010000001000100010001001011 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  404  	err |= TEST_ONE_SHIFT(1107575883, 2, s32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  405  	err |= TEST_ONE_SHIFT(1107575883, 2, int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  406  	/* 0100000100001000001000000010000001000010000001000100010001001011 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  407  	err |= TEST_ONE_SHIFT(4686030735197619275LL, 2, s64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  408  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  409  	/* Overflow: values larger than destination type. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  410  	err |= TEST_ONE_SHIFT(0x100, 0, u8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  411  	err |= TEST_ONE_SHIFT(0xFF, 0, s8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  412  	err |= TEST_ONE_SHIFT(0x10000U, 0, u16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  413  	err |= TEST_ONE_SHIFT(0xFFFFU, 0, s16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  414  	err |= TEST_ONE_SHIFT(0x100000000ULL, 0, u32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  415  	err |= TEST_ONE_SHIFT(0x100000000ULL, 0, unsigned int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  416  	err |= TEST_ONE_SHIFT(0xFFFFFFFFUL, 0, s32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  417  	err |= TEST_ONE_SHIFT(0xFFFFFFFFUL, 0, int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  418  	err |= TEST_ONE_SHIFT(0xFFFFFFFFFFFFFFFFULL, 0, s64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  419  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  420  	/* Nonsense: negative initial value. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  421  	err |= TEST_ONE_SHIFT(-1, 0, s8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  422  	err |= TEST_ONE_SHIFT(-1, 0, u8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  423  	err |= TEST_ONE_SHIFT(-5, 0, s16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  424  	err |= TEST_ONE_SHIFT(-5, 0, u16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  425  	err |= TEST_ONE_SHIFT(-10, 0, int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  426  	err |= TEST_ONE_SHIFT(-10, 0, unsigned int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  427  	err |= TEST_ONE_SHIFT(-100, 0, s32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  428  	err |= TEST_ONE_SHIFT(-100, 0, u32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  429  	err |= TEST_ONE_SHIFT(-10000, 0, s64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  430  	err |= TEST_ONE_SHIFT(-10000, 0, u64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  431  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  432  	/* Nonsense: negative shift values. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  433  	err |= TEST_ONE_SHIFT(0, -5, s8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  434  	err |= TEST_ONE_SHIFT(0, -5, u8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  435  	err |= TEST_ONE_SHIFT(0, -10, s16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  436  	err |= TEST_ONE_SHIFT(0, -10, u16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  437  	err |= TEST_ONE_SHIFT(0, -15, int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  438  	err |= TEST_ONE_SHIFT(0, -15, unsigned int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  439  	err |= TEST_ONE_SHIFT(0, -20, s32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  440  	err |= TEST_ONE_SHIFT(0, -20, u32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  441  	err |= TEST_ONE_SHIFT(0, -30, s64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  442  	err |= TEST_ONE_SHIFT(0, -30, u64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  443  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  444  	/* Overflow: shifted at or beyond entire type's bit width. */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  445  	err |= TEST_ONE_SHIFT(0, 8, u8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  446  	err |= TEST_ONE_SHIFT(0, 9, u8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  447  	err |= TEST_ONE_SHIFT(0, 8, s8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  448  	err |= TEST_ONE_SHIFT(0, 9, s8, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  449  	err |= TEST_ONE_SHIFT(0, 16, u16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  450  	err |= TEST_ONE_SHIFT(0, 17, u16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  451  	err |= TEST_ONE_SHIFT(0, 16, s16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  452  	err |= TEST_ONE_SHIFT(0, 17, s16, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  453  	err |= TEST_ONE_SHIFT(0, 32, u32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  454  	err |= TEST_ONE_SHIFT(0, 33, u32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  455  	err |= TEST_ONE_SHIFT(0, 32, int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  456  	err |= TEST_ONE_SHIFT(0, 33, int, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  457  	err |= TEST_ONE_SHIFT(0, 32, s32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  458  	err |= TEST_ONE_SHIFT(0, 33, s32, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  459  	err |= TEST_ONE_SHIFT(0, 64, u64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  460  	err |= TEST_ONE_SHIFT(0, 65, u64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  461  	err |= TEST_ONE_SHIFT(0, 64, s64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  462  	err |= TEST_ONE_SHIFT(0, 65, s64, 0, true);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  463  
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  464  	/*
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  465  	 * Corner case: for unsigned types, we fail when we've shifted
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  466  	 * through the entire width of bits. For signed types, we might
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  467  	 * want to match this behavior, but that would mean noticing if
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  468  	 * we shift through all but the signed bit, and this is not
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  469  	 * currently detected (but we'll notice an overflow into the
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  470  	 * signed bit). So, for now, we will test this condition but
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  471  	 * mark it as not expected to overflow.
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  472  	 */
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  473  	err |= TEST_ONE_SHIFT(0, 7, s8, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  474  	err |= TEST_ONE_SHIFT(0, 15, s16, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  475  	err |= TEST_ONE_SHIFT(0, 31, int, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  476  	err |= TEST_ONE_SHIFT(0, 31, s32, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  477  	err |= TEST_ONE_SHIFT(0, 63, s64, 0, false);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  478  
f1362e9519c80f lib/overflow-test.c Vitor Massaru Iha 2020-06-18  479  	KUNIT_EXPECT_FALSE(test, err);
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  480  }
d36b6ad27c7b95 lib/test_overflow.c Kees Cook         2018-08-01  481  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [RESEND, PATCH v2] lib: overflow-test: add KUnit test of check_*_overflow functions
  2020-06-19  3:05 ` Kees Cook
  2020-06-19 21:57   ` Vitor Massaru Iha
@ 2020-06-25 23:43   ` Brendan Higgins via Linux-kernel-mentees
  1 sibling, 0 replies; 5+ messages in thread
From: Brendan Higgins via Linux-kernel-mentees @ 2020-06-25 23:43 UTC (permalink / raw)
  To: Kees Cook
  Cc: David Gow, Rasmus Villemoes, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees,
	KUnit Development

On Thu, Jun 18, 2020 at 8:06 PM Kees Cook <keescook@chromium.org> wrote:

Apologies for just jumping in the middle of this late. Vitor just
brought something to my attention.

> On Thu, Jun 18, 2020 at 11:08:14AM -0300, Vitor Massaru Iha wrote:
> > This adds the conversion of the runtime tests of check_*_overflow functions,
> > from `lib/test_overflow.c`to KUnit tests.
> >
> > The log similar to the one seen in dmesg running test_overflow.c can be
> > seen in `test.log`.
> >
> > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > Tested-by: David Gow <davidgow@google.com>
> > ---
> > v2:
> >   * moved lib/test_overflow.c to lib/overflow-test.c;
>
> I still don't want a dash in the filename, as this creates a difference
> between the source name and the module name. While I still prefer
> overflow_kunit.c, I can get over it and accept overflow_test.c :)
>
> >     * back to original license;
> >     * fixed style code;
> >     * keeps __initconst and added _refdata on overflow_test_cases variable;
> >     * keeps macros intact making asserts with the variable err;
> >     * removed duplicate test_s8_overflow();
> >   * fixed typos on commit message;
> > ---
> >  lib/Kconfig.debug                        | 20 +++++++--
> >  lib/Makefile                             |  2 +-
> >  lib/{test_overflow.c => overflow-test.c} | 54 +++++++++---------------
> >  3 files changed, 38 insertions(+), 38 deletions(-)
> >  rename lib/{test_overflow.c => overflow-test.c} (96%)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index d74ac0fd6b2d..fb8a3955e969 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2000,9 +2000,6 @@ config TEST_UUID
> >  config TEST_XARRAY
> >       tristate "Test the XArray code at runtime"
> >
> > -config TEST_OVERFLOW
> > -     tristate "Test check_*_overflow() functions at runtime"
> > -
> >  config TEST_RHASHTABLE
> >       tristate "Perform selftest on resizable hash table"
> >       help
> > @@ -2155,6 +2152,23 @@ config SYSCTL_KUNIT_TEST
> >
> >         If unsure, say N.
> >
> > +config OVERFLOW_KUNIT_TEST
> > +     tristate "KUnit test for overflow" if !KUNIT_ALL_TESTS
> > +     depends on KUNIT
> > +     default KUNIT_ALL_TESTS
> > +     help
> > +       This builds the overflow KUnit tests.
> > +
> > +       KUnit tests run during boot and output the results to the debug log
> > +       in TAP format (http://testanything.org/). Only useful for kernel devs
> > +       running KUnit test harness and are not for inclusion into a production
> > +       build.
> > +
> > +       For more information on KUnit and unit tests in general please refer
> > +       to the KUnit documentation in Documentation/dev-tools/kunit/.
> > +
> > +       If unsure, say N.
> > +
> >  config LIST_KUNIT_TEST
> >       tristate "KUnit Test for Kernel Linked-list structures" if !KUNIT_ALL_TESTS
> >       depends on KUNIT
> > diff --git a/lib/Makefile b/lib/Makefile
> > index b1c42c10073b..3b725c9f92d4 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -75,7 +75,6 @@ obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
> >  obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
> >  obj-$(CONFIG_TEST_LKM) += test_module.o
> >  obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
> > -obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
> >  obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
> >  obj-$(CONFIG_TEST_SORT) += test_sort.o
> >  obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
> > @@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
> >  # KUnit tests
> >  obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> >  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> > +obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow-test.o
> > diff --git a/lib/test_overflow.c b/lib/overflow-test.c
> > similarity index 96%
> > rename from lib/test_overflow.c
> > rename to lib/overflow-test.c
> > index 7a4b6f6c5473..d40ef06b1ade 100644
> > --- a/lib/test_overflow.c
> > +++ b/lib/overflow-test.c
> > @@ -4,14 +4,11 @@
> >   */
> >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >
> > +#include <kunit/test.h>
> >  #include <linux/device.h>
> >  #include <linux/init.h>
> > -#include <linux/kernel.h>
> >  #include <linux/mm.h>
> > -#include <linux/module.h>
> >  #include <linux/overflow.h>
> > -#include <linux/slab.h>
> > -#include <linux/types.h>
> >  #include <linux/vmalloc.h>
> >
> >  #define DEFINE_TEST_ARRAY(t)                 \
> > @@ -270,7 +267,7 @@ DEFINE_TEST_FUNC(u64, "%llu");
> >  DEFINE_TEST_FUNC(s64, "%lld");
> >  #endif
> >
> > -static int __init test_overflow_calculation(void)
> > +static void __init overflow_calculation_test(struct kunit *test)
> >  {
> >       int err = 0;
> >
> > @@ -285,10 +282,10 @@ static int __init test_overflow_calculation(void)
> >       err |= test_s64_overflow();
> >  #endif
> >
> > -     return err;
> > +     KUNIT_EXPECT_FALSE(test, err);
> >  }
>
> Ah! Well, yes, I guess that is one way to do it. :) I'm just curious:
> why the change away from doing EXPECTs on individual tests?
>
> >
> > -static int __init test_overflow_shift(void)
> > +static void __init overflow_shift_test(struct kunit *test)
> >  {
> >       int err = 0;
> >
> > @@ -479,7 +476,7 @@ static int __init test_overflow_shift(void)
> >       err |= TEST_ONE_SHIFT(0, 31, s32, 0, false);
> >       err |= TEST_ONE_SHIFT(0, 63, s64, 0, false);
> >
> > -     return err;
> > +     KUNIT_EXPECT_FALSE(test, err);
> >  }
> >
> >  /*
> > @@ -555,7 +552,7 @@ DEFINE_TEST_ALLOC(kvzalloc_node, kvfree,     0, 1, 1);
> >  DEFINE_TEST_ALLOC(devm_kmalloc,  devm_kfree, 1, 1, 0);
> >  DEFINE_TEST_ALLOC(devm_kzalloc,  devm_kfree, 1, 1, 0);
> >
> > -static int __init test_overflow_allocation(void)
> > +static void __init overflow_allocation_test(struct kunit *test)
> >  {
> >       const char device_name[] = "overflow-test";
> >       struct device *dev;
> > @@ -563,10 +560,8 @@ static int __init test_overflow_allocation(void)
> >
> >       /* Create dummy device for devm_kmalloc()-family tests. */
> >       dev = root_device_register(device_name);
> > -     if (IS_ERR(dev)) {
> > -             pr_warn("Cannot register test device\n");
> > -             return 1;
> > -     }
> > +     if (IS_ERR(dev))
> > +             kunit_warn(test, "Cannot register test device\n");
> >
> >       err |= test_kmalloc(NULL);
> >       err |= test_kmalloc_node(NULL);
> > @@ -585,30 +580,21 @@ static int __init test_overflow_allocation(void)
> >
> >       device_unregister(dev);
> >
> > -     return err;
> > +     KUNIT_EXPECT_FALSE(test, err);
> >  }
> >
> > -static int __init test_module_init(void)
> > -{
> > -     int err = 0;
> > -
> > -     err |= test_overflow_calculation();
> > -     err |= test_overflow_shift();
> > -     err |= test_overflow_allocation();
> > -
> > -     if (err) {
> > -             pr_warn("FAIL!\n");
> > -             err = -EINVAL;
> > -     } else {
> > -             pr_info("all tests passed\n");
> > -     }
> > +static struct kunit_case __refdata overflow_test_cases[] = {
>
> Erm, __refdata? This seems like it should be __initdata.
>
> > +     KUNIT_CASE(overflow_calculation_test),
> > +     KUNIT_CASE(overflow_shift_test),
> > +     KUNIT_CASE(overflow_allocation_test),
> > +     {}
> > +};
> >
> > -     return err;
> > -}
> > +static struct kunit_suite overflow_test_suite = {
>
> And this.
>
> > +     .name = "overflow",
> > +     .test_cases = overflow_test_cases,
> > +};
> >
> > -static void __exit test_module_exit(void)
> > -{ }
> > +kunit_test_suites(&overflow_test_suite);
>
> I suspect the problem causing the need for __refdata there is the lack
> of __init markings on the functions in kunit_test_suites()?

That is correct, Kees.

This problem goes all the way back to the initial KUnit RFC. I forget
who first brought it up, but there was an issue that KUnit runs in
init and consequently has access to __init stuff, but we weren't sure
that we want to mark *everything* in KUnit as __init. This was for a
number of reasons: one of these reasons was that we were considering
allowing KUnit to run at different times; with Alan's KUNIT_DEBUGFS
patches, this is now a reality. Thus, in some configurations, KUnit
can run its tests after kernel init is done, but also not as a module.

Sorry for not seeing this before.

> (Or maybe this is explained somewhere else I've missed it.)
>
> For example, would this work? (I haven't tested it all.)
>
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 59f3144f009a..aad746d59d2f 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -233,9 +233,9 @@ size_t kunit_suite_num_test_cases(struct kunit_suite *suite);
>  unsigned int kunit_test_case_num(struct kunit_suite *suite,
>                                  struct kunit_case *test_case);
>
> -int __kunit_test_suites_init(struct kunit_suite **suites);
> +int __init __kunit_test_suites_init(struct kunit_suite **suites);
>
> -void __kunit_test_suites_exit(struct kunit_suite **suites);
> +void __exit __kunit_test_suites_exit(struct kunit_suite **suites);
>
>  /**
>   * kunit_test_suites() - used to register one or more &struct kunit_suite
> @@ -263,8 +263,9 @@ void __kunit_test_suites_exit(struct kunit_suite **suites);
>   * everything else is definitely initialized.
>   */
>  #define kunit_test_suites(suites_list...)                              \
> -       static struct kunit_suite *suites[] = {suites_list, NULL};      \
> -       static int kunit_test_suites_init(void)                         \
> +       static struct kunit_suite *suites[] __initdata =                \
> +               {suites_list, NULL};                                    \
> +       static int __init kunit_test_suites_init(void)                  \
>         {                                                               \
>                 return __kunit_test_suites_init(suites);                \
>         }                                                               \
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index c36037200310..bfb0f563721b 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -381,7 +381,7 @@ static void kunit_init_suite(struct kunit_suite *suite)
>         kunit_debugfs_create_suite(suite);
>  }
>
> -int __kunit_test_suites_init(struct kunit_suite **suites)
> +int __init __kunit_test_suites_init(struct kunit_suite **suites)
>  {
>         unsigned int i;
>
> @@ -393,7 +393,7 @@ int __kunit_test_suites_init(struct kunit_suite **suites)
>  }
>  EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
>
> -static void kunit_exit_suite(struct kunit_suite *suite)
> +static void __exit kunit_exit_suite(struct kunit_suite *suite)
>  {
>         kunit_debugfs_destroy_suite(suite);
>  }
>
> >
> > -module_init(test_module_init);
> > -module_exit(test_module_exit);
> >  MODULE_LICENSE("Dual MIT/GPL");
> >
> > base-commit: 7bf200b3a4ac10b1b0376c70b8c66ed39eae7cdd
> > prerequisite-patch-id: e827b6b22f950b9f69620805a04e4a264cf4cc6a
> > --
> > 2.26.2
> >
>
> Thanks again for the conversion!
>
> --
> Kees Cook
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

end of thread, other threads:[~2020-06-25 23:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-18 14:08 [Linux-kernel-mentees] [RESEND, PATCH v2] lib: overflow-test: add KUnit test of check_*_overflow functions Vitor Massaru Iha
2020-06-19  3:05 ` Kees Cook
2020-06-19 21:57   ` Vitor Massaru Iha
2020-06-25 23:43   ` Brendan Higgins via Linux-kernel-mentees
2020-06-25 13:58 ` kernel test robot

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