[PATCH 1/2] kunit: support failure from dynamic analysis tools

[PATCH 1/2] kunit: support failure from dynamic analysis tools

[Uriel Guajardo]

Adds an API to allow dynamic analysis tools to fail the currently
running KUnit test case.

- Always places the kunit test in the task_struct to allow other tools
to access the currently running KUnit test.

- Creates a new header file to avoid circular dependencies that could be
created from the test.h file.

Requires KASAN-KUnit integration patch to access the kunit test from
task_struct:
https://lore.kernel.org/linux-kselftest/20200606040349.246780-2-davidgow@google.com/

Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
---
 include/kunit/test-bug.h | 24 ++++++++++++++++++++++++
 include/kunit/test.h     |  1 +
 lib/kunit/test.c         | 10 ++++++----
 3 files changed, 31 insertions(+), 4 deletions(-)
 create mode 100644 include/kunit/test-bug.h

diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
new file mode 100644
index 000000000000..283c19ec328f
--- /dev/null
+++ b/include/kunit/test-bug.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit API allowing dynamic analysis tools to interact with KUnit tests
+ *
+ * Copyright (C) 2020, Google LLC.
+ * Author: Uriel Guajardo <urielguajardo@google.com>
+ */
+
+#ifndef _KUNIT_TEST_BUG_H
+#define _KUNIT_TEST_BUG_H
+
+#if IS_ENABLED(CONFIG_KUNIT)
+
+extern void kunit_fail_current_test(void);
+
+#else
+
+static inline void kunit_fail_current_test(void)
+{
+}
+
+#endif
+
+#endif /* _KUNIT_TEST_BUG_H */
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 3391f38389f8..81bf43a1abda 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -11,6 +11,7 @@
 
 #include <kunit/assert.h>
 #include <kunit/try-catch.h>
+#include <kunit/test-bug.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index dcc35fd30d95..d8189d827368 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -16,6 +16,12 @@
 #include "string-stream.h"
 #include "try-catch-impl.h"
 
+void kunit_fail_current_test(void)
+{
+	if (current->kunit_test)
+		kunit_set_failure(current->kunit_test);
+}
+
 static void kunit_print_tap_version(void)
 {
 	static bool kunit_has_printed_tap_version;
@@ -284,9 +290,7 @@ static void kunit_try_run_case(void *data)
 	struct kunit_suite *suite = ctx->suite;
 	struct kunit_case *test_case = ctx->test_case;
 
-#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
 	current->kunit_test = test;
-#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */
 
 	/*
 	 * kunit_run_case_internal may encounter a fatal error; if it does,
@@ -602,9 +606,7 @@ void kunit_cleanup(struct kunit *test)
 		spin_unlock(&test->lock);
 		kunit_remove_resource(test, res);
 	}
-#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
 	current->kunit_test = NULL;
-#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT)*/
 }
 EXPORT_SYMBOL_GPL(kunit_cleanup);
 
-- 
2.28.0.163.g6104cc2f0b6-goog

[PATCH 2/2] kunit: ubsan integration

[Uriel Guajardo]

Integrates UBSAN into the KUnit testing framework. It fails KUnit tests
whenever it reports undefined behavior.

Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
---
 lib/ubsan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ubsan.c b/lib/ubsan.c
index cb9af3f6b77e..1460e2c828c8 100644
--- a/lib/ubsan.c
+++ b/lib/ubsan.c
@@ -14,6 +14,7 @@
 #include <linux/types.h>
 #include <linux/sched.h>
 #include <linux/uaccess.h>
+#include <kunit/test.h>
 
 #include "ubsan.h"
 
@@ -137,6 +138,7 @@ static void ubsan_prologue(struct source_location *loc, const char *reason)
 {
 	current->in_ubsan++;
 
+	kunit_fail_current_test();
 	pr_err("========================================"
 		"========================================\n");
 	pr_err("UBSAN: %s in %s:%d:%d\n", reason, loc->file_name,
-- 
2.28.0.163.g6104cc2f0b6-goog

Re: [PATCH 2/2] kunit: ubsan integration

[Brendan Higgins]

On Thu, Aug 6, 2020 at 10:43 AM Uriel Guajardo
<urielguajardojr@gmail.com> wrote:
>
> Integrates UBSAN into the KUnit testing framework. It fails KUnit tests
> whenever it reports undefined behavior.
>
> Signed-off-by: Uriel Guajardo <urielguajardo@google.com>

You should resend this to the UBSAN maintainers as well; they will
need to sign off on this.

In the future, make sure to run get_maintainers.pl

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

> ---
>  lib/ubsan.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/lib/ubsan.c b/lib/ubsan.c
> index cb9af3f6b77e..1460e2c828c8 100644
> --- a/lib/ubsan.c
> +++ b/lib/ubsan.c
> @@ -14,6 +14,7 @@
>  #include <linux/types.h>
>  #include <linux/sched.h>
>  #include <linux/uaccess.h>
> +#include <kunit/test.h>
>
>  #include "ubsan.h"
>
> @@ -137,6 +138,7 @@ static void ubsan_prologue(struct source_location *loc, const char *reason)
>  {
>         current->in_ubsan++;
>
> +       kunit_fail_current_test();
>         pr_err("========================================"
>                 "========================================\n");
>         pr_err("UBSAN: %s in %s:%d:%d\n", reason, loc->file_name,
> --
> 2.28.0.163.g6104cc2f0b6-goog
>

Re: [PATCH 2/2] kunit: ubsan integration

[Uriel Guajardo]

On Mon, Aug 10, 2020 at 3:43 PM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> On Thu, Aug 6, 2020 at 10:43 AM Uriel Guajardo
> <urielguajardojr@gmail.com> wrote:
> >
> > Integrates UBSAN into the KUnit testing framework. It fails KUnit tests
> > whenever it reports undefined behavior.
> >
> > Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
>
> You should resend this to the UBSAN maintainers as well; they will
> need to sign off on this.
>
> In the future, make sure to run get_maintainers.pl

There are no listed UBSAN maintainers. Will instead resend to the
suggestions from get_maintainers.

>
> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
>
> > ---
> >  lib/ubsan.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/ubsan.c b/lib/ubsan.c
> > index cb9af3f6b77e..1460e2c828c8 100644
> > --- a/lib/ubsan.c
> > +++ b/lib/ubsan.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/types.h>
> >  #include <linux/sched.h>
> >  #include <linux/uaccess.h>
> > +#include <kunit/test.h>
> >
> >  #include "ubsan.h"
> >
> > @@ -137,6 +138,7 @@ static void ubsan_prologue(struct source_location *loc, const char *reason)
> >  {
> >         current->in_ubsan++;
> >
> > +       kunit_fail_current_test();
> >         pr_err("========================================"
> >                 "========================================\n");
> >         pr_err("UBSAN: %s in %s:%d:%d\n", reason, loc->file_name,
> > --
> > 2.28.0.163.g6104cc2f0b6-goog
> >

Re: [PATCH 2/2] kunit: ubsan integration

[Qian Cai]

On Thu, Aug 06, 2020 at 05:43:26PM +0000, Uriel Guajardo wrote:
> Integrates UBSAN into the KUnit testing framework. It fails KUnit tests
> whenever it reports undefined behavior.
> 
> Signed-off-by: Uriel Guajardo <urielguajardo@google.com>

It looks like this patch had been merged into linux-next but the "[PATCH 1/2]
kunit: support failure from dynamic analysis tools" did not. Hence, it caused a
compiling failure.

lib/ubsan.c: In function ‘ubsan_prologue’:
lib/ubsan.c:141:2: error: implicit declaration of function ‘kunit_fail_current_test’; did you mean ‘kunit_init_test’? [-Werror=implicit-function-declaration]
  kunit_fail_current_test();
  ^~~~~~~~~~~~~~~~~~~~~~~
  kunit_init_test
cc1: some warnings being treated as errors

> ---
>  lib/ubsan.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/ubsan.c b/lib/ubsan.c
> index cb9af3f6b77e..1460e2c828c8 100644
> --- a/lib/ubsan.c
> +++ b/lib/ubsan.c
> @@ -14,6 +14,7 @@
>  #include <linux/types.h>
>  #include <linux/sched.h>
>  #include <linux/uaccess.h>
> +#include <kunit/test.h>
>  
>  #include "ubsan.h"
>  
> @@ -137,6 +138,7 @@ static void ubsan_prologue(struct source_location *loc, const char *reason)
>  {
>  	current->in_ubsan++;
>  
> +	kunit_fail_current_test();
>  	pr_err("========================================"
>  		"========================================\n");
>  	pr_err("UBSAN: %s in %s:%d:%d\n", reason, loc->file_name,
> -- 
> 2.28.0.163.g6104cc2f0b6-goog
> 

Re: [PATCH 2/2] kunit: ubsan integration

[Qian Cai]

On Wed, Sep 02, 2020 at 08:52:24AM -0400, Qian Cai wrote:
> On Thu, Aug 06, 2020 at 05:43:26PM +0000, Uriel Guajardo wrote:
> > Integrates UBSAN into the KUnit testing framework. It fails KUnit tests
> > whenever it reports undefined behavior.
> > 
> > Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
> 
> It looks like this patch had been merged into linux-next but the "[PATCH 1/2]
> kunit: support failure from dynamic analysis tools" did not. Hence, it caused a
> compiling failure.
> 
> lib/ubsan.c: In function ‘ubsan_prologue’:
> lib/ubsan.c:141:2: error: implicit declaration of function ‘kunit_fail_current_test’; did you mean ‘kunit_init_test’? [-Werror=implicit-function-declaration]
>   kunit_fail_current_test();
>   ^~~~~~~~~~~~~~~~~~~~~~~
>   kunit_init_test
> cc1: some warnings being treated as errors

Stephen, Shuah, can you revert this commit or pick up its dependency as well?

https://lore.kernel.org/linux-kselftest/20200813205722.1384108-1-urielguajardojr@gmail.com/

Still seeing this on today's linux-next build.

> 
> > ---
> >  lib/ubsan.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/lib/ubsan.c b/lib/ubsan.c
> > index cb9af3f6b77e..1460e2c828c8 100644
> > --- a/lib/ubsan.c
> > +++ b/lib/ubsan.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/types.h>
> >  #include <linux/sched.h>
> >  #include <linux/uaccess.h>
> > +#include <kunit/test.h>
> >  
> >  #include "ubsan.h"
> >  
> > @@ -137,6 +138,7 @@ static void ubsan_prologue(struct source_location *loc, const char *reason)
> >  {
> >  	current->in_ubsan++;
> >  
> > +	kunit_fail_current_test();
> >  	pr_err("========================================"
> >  		"========================================\n");
> >  	pr_err("UBSAN: %s in %s:%d:%d\n", reason, loc->file_name,
> > -- 
> > 2.28.0.163.g6104cc2f0b6-goog
> > 

Re: [PATCH 2/2] kunit: ubsan integration

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]

Hi Qian,

On Thu, 3 Sep 2020 08:38:25 -0400 Qian Cai <cai@lca.pw> wrote:
>
> On Wed, Sep 02, 2020 at 08:52:24AM -0400, Qian Cai wrote:
> > On Thu, Aug 06, 2020 at 05:43:26PM +0000, Uriel Guajardo wrote:  
> > > Integrates UBSAN into the KUnit testing framework. It fails KUnit tests
> > > whenever it reports undefined behavior.
> > > 
> > > Signed-off-by: Uriel Guajardo <urielguajardo@google.com>  
> > 
> > It looks like this patch had been merged into linux-next but the "[PATCH 1/2]
> > kunit: support failure from dynamic analysis tools" did not. Hence, it caused a
> > compiling failure.
> > 
> > lib/ubsan.c: In function ‘ubsan_prologue’:
> > lib/ubsan.c:141:2: error: implicit declaration of function ‘kunit_fail_current_test’; did you mean ‘kunit_init_test’? [-Werror=implicit-function-declaration]
> >   kunit_fail_current_test();
> >   ^~~~~~~~~~~~~~~~~~~~~~~
> >   kunit_init_test
> > cc1: some warnings being treated as errors  
> 
> Stephen, Shuah, can you revert this commit or pick up its dependency as well?

Sorry, for the slow response ... this should be gone today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] kunit: support failure from dynamic analysis tools

[Alan Maguire]

On Thu, 6 Aug 2020, Uriel Guajardo wrote:

> Adds an API to allow dynamic analysis tools to fail the currently
> running KUnit test case.
> 
> - Always places the kunit test in the task_struct to allow other tools
> to access the currently running KUnit test.
> 
> - Creates a new header file to avoid circular dependencies that could be
> created from the test.h file.
> 
> Requires KASAN-KUnit integration patch to access the kunit test from
> task_struct:
> https://lore.kernel.org/linux-kselftest/20200606040349.246780-2-davidgow@google.com/
> 
> Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
> ---
>  include/kunit/test-bug.h | 24 ++++++++++++++++++++++++
>  include/kunit/test.h     |  1 +
>  lib/kunit/test.c         | 10 ++++++----
>  3 files changed, 31 insertions(+), 4 deletions(-)
>  create mode 100644 include/kunit/test-bug.h
> 
> diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
> new file mode 100644
> index 000000000000..283c19ec328f
> --- /dev/null
> +++ b/include/kunit/test-bug.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * KUnit API allowing dynamic analysis tools to interact with KUnit tests
> + *
> + * Copyright (C) 2020, Google LLC.
> + * Author: Uriel Guajardo <urielguajardo@google.com>
> + */
> +
> +#ifndef _KUNIT_TEST_BUG_H
> +#define _KUNIT_TEST_BUG_H
> +
> +#if IS_ENABLED(CONFIG_KUNIT)
> +
> +extern void kunit_fail_current_test(void);
> +
> +#else
> +
> +static inline void kunit_fail_current_test(void)
> +{
> +}
> +
> +#endif
> +
> +#endif /* _KUNIT_TEST_BUG_H */

This is great stuff!

One thing I wonder though; how obvious will it be to someone
running a KUnit test that the cause of the test failure
is a dynamic analysis tool?  Yes we'll see the dmesg logging
from that tool but I don't think there's any context _within_
KUnit that could clarify the source of the failure.  What about
changing the above API to include a string message that KUnit can
log, so it can at least identify the source of the failure
(ubsan, kasan etc).  That would alert anyone looking at KUnit
output only that there's an external context to examine.

> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 3391f38389f8..81bf43a1abda 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -11,6 +11,7 @@
>  
>  #include <kunit/assert.h>
>  #include <kunit/try-catch.h>
> +#include <kunit/test-bug.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index dcc35fd30d95..d8189d827368 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -16,6 +16,12 @@
>  #include "string-stream.h"
>  #include "try-catch-impl.h"
>  
> +void kunit_fail_current_test(void)
> +{
> +	if (current->kunit_test)
> +		kunit_set_failure(current->kunit_test);
> +}
> +
>  static void kunit_print_tap_version(void)
>  {
>  	static bool kunit_has_printed_tap_version;
> @@ -284,9 +290,7 @@ static void kunit_try_run_case(void *data)
>  	struct kunit_suite *suite = ctx->suite;
>  	struct kunit_case *test_case = ctx->test_case;
>  
> -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
>  	current->kunit_test = test;
> -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */
>  
>  	/*
>  	 * kunit_run_case_internal may encounter a fatal error; if it does,
> @@ -602,9 +606,7 @@ void kunit_cleanup(struct kunit *test)
>  		spin_unlock(&test->lock);
>  		kunit_remove_resource(test, res);
>  	}
> -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
>  	current->kunit_test = NULL;
> -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT)*/
>  }
>  EXPORT_SYMBOL_GPL(kunit_cleanup);
>  
> -- 
> 2.28.0.163.g6104cc2f0b6-goog
> 
> 

Re: [PATCH 1/2] kunit: support failure from dynamic analysis tools

[Uriel Guajardo]

On Fri, Aug 7, 2020 at 9:16 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Thu, 6 Aug 2020, Uriel Guajardo wrote:
>
> > Adds an API to allow dynamic analysis tools to fail the currently
> > running KUnit test case.
> >
> > - Always places the kunit test in the task_struct to allow other tools
> > to access the currently running KUnit test.
> >
> > - Creates a new header file to avoid circular dependencies that could be
> > created from the test.h file.
> >
> > Requires KASAN-KUnit integration patch to access the kunit test from
> > task_struct:
> > https://lore.kernel.org/linux-kselftest/20200606040349.246780-2-davidgow@google.com/
> >
> > Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
> > ---
> >  include/kunit/test-bug.h | 24 ++++++++++++++++++++++++
> >  include/kunit/test.h     |  1 +
> >  lib/kunit/test.c         | 10 ++++++----
> >  3 files changed, 31 insertions(+), 4 deletions(-)
> >  create mode 100644 include/kunit/test-bug.h
> >
> > diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
> > new file mode 100644
> > index 000000000000..283c19ec328f
> > --- /dev/null
> > +++ b/include/kunit/test-bug.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * KUnit API allowing dynamic analysis tools to interact with KUnit tests
> > + *
> > + * Copyright (C) 2020, Google LLC.
> > + * Author: Uriel Guajardo <urielguajardo@google.com>
> > + */
> > +
> > +#ifndef _KUNIT_TEST_BUG_H
> > +#define _KUNIT_TEST_BUG_H
> > +
> > +#if IS_ENABLED(CONFIG_KUNIT)
> > +
> > +extern void kunit_fail_current_test(void);
> > +
> > +#else
> > +
> > +static inline void kunit_fail_current_test(void)
> > +{
> > +}
> > +
> > +#endif
> > +
> > +#endif /* _KUNIT_TEST_BUG_H */
>
> This is great stuff!
>
> One thing I wonder though; how obvious will it be to someone
> running a KUnit test that the cause of the test failure
> is a dynamic analysis tool?  Yes we'll see the dmesg logging
> from that tool but I don't think there's any context _within_
> KUnit that could clarify the source of the failure.  What about
> changing the above API to include a string message that KUnit can
> log, so it can at least identify the source of the failure
> (ubsan, kasan etc).  That would alert anyone looking at KUnit
> output only that there's an external context to examine.
>

Good point! You're right: as it stands, there is no context within
KUnit indicating the source of the failure, and the tool itself is
responsible for logging. This patch is mainly focused on just failing
test cases from outside KUnit.

I'm actually working on sending a follow-up patch soon that supports
the expectation of failures from specific tools. It will introduce
something similar to what you are suggesting, so that KUnit can
properly distinguish between different failures and log them
appropriately. Thanks for the suggestion!




> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index 3391f38389f8..81bf43a1abda 100644
> > --- a/include/kunit/test.h
> > +++ b/include/kunit/test.h
> > @@ -11,6 +11,7 @@
> >
> >  #include <kunit/assert.h>
> >  #include <kunit/try-catch.h>
> > +#include <kunit/test-bug.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/slab.h>
> > diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> > index dcc35fd30d95..d8189d827368 100644
> > --- a/lib/kunit/test.c
> > +++ b/lib/kunit/test.c
> > @@ -16,6 +16,12 @@
> >  #include "string-stream.h"
> >  #include "try-catch-impl.h"
> >
> > +void kunit_fail_current_test(void)
> > +{
> > +     if (current->kunit_test)
> > +             kunit_set_failure(current->kunit_test);
> > +}
> > +
> >  static void kunit_print_tap_version(void)
> >  {
> >       static bool kunit_has_printed_tap_version;
> > @@ -284,9 +290,7 @@ static void kunit_try_run_case(void *data)
> >       struct kunit_suite *suite = ctx->suite;
> >       struct kunit_case *test_case = ctx->test_case;
> >
> > -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
> >       current->kunit_test = test;
> > -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */
> >
> >       /*
> >        * kunit_run_case_internal may encounter a fatal error; if it does,
> > @@ -602,9 +606,7 @@ void kunit_cleanup(struct kunit *test)
> >               spin_unlock(&test->lock);
> >               kunit_remove_resource(test, res);
> >       }
> > -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
> >       current->kunit_test = NULL;
> > -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT)*/
> >  }
> >  EXPORT_SYMBOL_GPL(kunit_cleanup);
> >
> > --
> > 2.28.0.163.g6104cc2f0b6-goog
> >
> >

Re: [PATCH 1/2] kunit: support failure from dynamic analysis tools

[Brendan Higgins]

On Thu, Aug 6, 2020 at 10:43 AM Uriel Guajardo
<urielguajardojr@gmail.com> wrote:
>
> Adds an API to allow dynamic analysis tools to fail the currently
> running KUnit test case.
>
> - Always places the kunit test in the task_struct to allow other tools
> to access the currently running KUnit test.
>
> - Creates a new header file to avoid circular dependencies that could be
> created from the test.h file.
>
> Requires KASAN-KUnit integration patch to access the kunit test from
> task_struct:
> https://lore.kernel.org/linux-kselftest/20200606040349.246780-2-davidgow@google.com/
>
> Signed-off-by: Uriel Guajardo <urielguajardo@google.com>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

[PATCH 1/2] kunit: support failure from dynamic analysis tools

[Uriel Guajardo]

Adds an API to allow dynamic analysis tools to fail the currently
running KUnit test case.

- Always places the kunit test in the task_struct to allow other tools
to access the currently running KUnit test.

- Creates a new header file to avoid circular dependencies that could be
created from the test.h file.

Requires KASAN-KUnit integration patch to access the kunit test from
task_struct:
https://lore.kernel.org/linux-kselftest/20200606040349.246780-2-davidgow@google.com/

Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
---
 include/kunit/test-bug.h | 24 ++++++++++++++++++++++++
 include/kunit/test.h     |  1 +
 lib/kunit/test.c         | 10 ++++++----
 3 files changed, 31 insertions(+), 4 deletions(-)
 create mode 100644 include/kunit/test-bug.h

diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
new file mode 100644
index 000000000000..283c19ec328f
--- /dev/null
+++ b/include/kunit/test-bug.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit API allowing dynamic analysis tools to interact with KUnit tests
+ *
+ * Copyright (C) 2020, Google LLC.
+ * Author: Uriel Guajardo <urielguajardo@google.com>
+ */
+
+#ifndef _KUNIT_TEST_BUG_H
+#define _KUNIT_TEST_BUG_H
+
+#if IS_ENABLED(CONFIG_KUNIT)
+
+extern void kunit_fail_current_test(void);
+
+#else
+
+static inline void kunit_fail_current_test(void)
+{
+}
+
+#endif
+
+#endif /* _KUNIT_TEST_BUG_H */
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 3391f38389f8..81bf43a1abda 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -11,6 +11,7 @@
 
 #include <kunit/assert.h>
 #include <kunit/try-catch.h>
+#include <kunit/test-bug.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index dcc35fd30d95..d8189d827368 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -16,6 +16,12 @@
 #include "string-stream.h"
 #include "try-catch-impl.h"
 
+void kunit_fail_current_test(void)
+{
+	if (current->kunit_test)
+		kunit_set_failure(current->kunit_test);
+}
+
 static void kunit_print_tap_version(void)
 {
 	static bool kunit_has_printed_tap_version;
@@ -284,9 +290,7 @@ static void kunit_try_run_case(void *data)
 	struct kunit_suite *suite = ctx->suite;
 	struct kunit_case *test_case = ctx->test_case;
 
-#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
 	current->kunit_test = test;
-#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */
 
 	/*
 	 * kunit_run_case_internal may encounter a fatal error; if it does,
@@ -602,9 +606,7 @@ void kunit_cleanup(struct kunit *test)
 		spin_unlock(&test->lock);
 		kunit_remove_resource(test, res);
 	}
-#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
 	current->kunit_test = NULL;
-#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT)*/
 }
 EXPORT_SYMBOL_GPL(kunit_cleanup);
 
-- 
2.28.0.163.g6104cc2f0b6-goog

Re: [PATCH 1/2] kunit: support failure from dynamic analysis tools

[Shuah Khan]

On 8/13/20 2:57 PM, Uriel Guajardo wrote:
> Adds an API to allow dynamic analysis tools to fail the currently
> running KUnit test case.
> 
> - Always places the kunit test in the task_struct to allow other tools
> to access the currently running KUnit test.
> 
> - Creates a new header file to avoid circular dependencies that could be
> created from the test.h file.
> 
> Requires KASAN-KUnit integration patch to access the kunit test from
> task_struct:
> https://lore.kernel.org/linux-kselftest/20200606040349.246780-2-davidgow@google.com/
> 
> Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
> ---
>   include/kunit/test-bug.h | 24 ++++++++++++++++++++++++
>   include/kunit/test.h     |  1 +
>   lib/kunit/test.c         | 10 ++++++----
>   3 files changed, 31 insertions(+), 4 deletions(-)
>   create mode 100644 include/kunit/test-bug.h
> 
> diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
> new file mode 100644
> index 000000000000..283c19ec328f
> --- /dev/null
> +++ b/include/kunit/test-bug.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * KUnit API allowing dynamic analysis tools to interact with KUnit tests
> + *
> + * Copyright (C) 2020, Google LLC.
> + * Author: Uriel Guajardo <urielguajardo@google.com>
> + */
> +
> +#ifndef _KUNIT_TEST_BUG_H
> +#define _KUNIT_TEST_BUG_H
> +
> +#if IS_ENABLED(CONFIG_KUNIT)
> +
> +extern void kunit_fail_current_test(void);
> +
> +#else
> +
> +static inline void kunit_fail_current_test(void)
> +{
> +}
> +
> +#endif
> +
> +#endif /* _KUNIT_TEST_BUG_H */
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 3391f38389f8..81bf43a1abda 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -11,6 +11,7 @@
>   
>   #include <kunit/assert.h>
>   #include <kunit/try-catch.h>
> +#include <kunit/test-bug.h>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/slab.h>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index dcc35fd30d95..d8189d827368 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -16,6 +16,12 @@
>   #include "string-stream.h"
>   #include "try-catch-impl.h"
>   
> +void kunit_fail_current_test(void)
> +{
> +	if (current->kunit_test)
> +		kunit_set_failure(current->kunit_test);
> +}
> +
>   static void kunit_print_tap_version(void)
>   {
>   	static bool kunit_has_printed_tap_version;
> @@ -284,9 +290,7 @@ static void kunit_try_run_case(void *data)
>   	struct kunit_suite *suite = ctx->suite;
>   	struct kunit_case *test_case = ctx->test_case;
>   
> -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
>   	current->kunit_test = test;
> -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */
>   
>   	/*
>   	 * kunit_run_case_internal may encounter a fatal error; if it does,
> @@ -602,9 +606,7 @@ void kunit_cleanup(struct kunit *test)
>   		spin_unlock(&test->lock);
>   		kunit_remove_resource(test, res);
>   	}
> -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
>   	current->kunit_test = NULL;
> -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT)*/
>   }
>   EXPORT_SYMBOL_GPL(kunit_cleanup);
>   
> 

Is this patch still needed. Doesn't apply on top of Linux 5.9. From
q quick review, doesn't looks like it is needed. Not seeing any
CONFIG_KASAN's in lib/kunit/test.c

Here is the .rej

--- lib/kunit/test.c
+++ lib/kunit/test.c
@@ -290,9 +296,7 @@ static void kunit_try_run_case(void *data)
         struct kunit_suite *suite = ctx->suite;
         struct kunit_case *test_case = ctx->test_case;

-#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
         current->kunit_test = test;
-#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */

         /*
          * kunit_run_case_internal may encounter a fatal error; if it does,
@@ -608,9 +612,7 @@ void kunit_cleanup(struct kunit *test)
                 spin_unlock(&test->lock);
                 kunit_remove_resource(test, res);
         }
-#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
         current->kunit_test = NULL;
-#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT)*/
  }
  EXPORT_SYMBOL_GPL(kunit_cleanup);

If this is still needed, please send me rebased patch.

thanks,
-- Shuah