From: Dmitry Vyukov <dvyukov@google.com>
To: Patricia Alfonso <trishalfonso@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>,
Brendan Higgins <brendanhiggins@google.com>,
David Gow <davidgow@google.com>, Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
LKML <linux-kernel@vger.kernel.org>,
kasan-dev <kasan-dev@googlegroups.com>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>,
kunit-dev@googlegroups.com
Subject: Re: [RFC PATCH 2/2] KUnit: KASAN Integration
Date: Wed, 4 Mar 2020 07:35:02 +0100 [thread overview]
Message-ID: <CACT4Y+avZ5ZdTUPQy7as3PAvdYd0NcSsyUukiumHa_Ah9ZYCFA@mail.gmail.com> (raw)
In-Reply-To: <20200227024301.217042-2-trishalfonso@google.com>
On Thu, Feb 27, 2020 at 3:44 AM 'Patricia Alfonso' via kasan-dev
<kasan-dev@googlegroups.com> wrote:
>
> Integrate KASAN into KUnit testing framework.
> - Fail tests when KASAN reports an error that is not expected
> - Use KUNIT_EXPECT_KASAN_FAIL to expect a KASAN error in KASAN tests
> - KUnit struct added to current task to keep track of the current test
> from KASAN code
> - Booleans representing if a KASAN report is expected and if a KASAN
> report is found added to kunit struct
> - This prints "line# has passed" or "line# has failed"
>
> Signed-off-by: Patricia Alfonso <trishalfonso@google.com>
> ---
> If anyone has any suggestions on how best to print the failure
> messages, please share!
>
> One issue I have found while testing this is the allocation fails in
> kmalloc_pagealloc_oob_right() sometimes, but not consistently. This
> does cause the test to fail on the KUnit side, as expected, but it
> seems to skip all the tests before this one because the output starts
> with this failure instead of with the first test, kmalloc_oob_right().
>
> include/kunit/test.h | 24 ++++++++++++++++++++++++
> include/linux/sched.h | 7 ++++++-
> lib/kunit/test.c | 7 ++++++-
> mm/kasan/report.c | 19 +++++++++++++++++++
> tools/testing/kunit/kunit_kernel.py | 2 +-
> 5 files changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 2dfb550c6723..2e388f8937f3 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -21,6 +21,8 @@ struct kunit_resource;
> typedef int (*kunit_resource_init_t)(struct kunit_resource *, void *);
> typedef void (*kunit_resource_free_t)(struct kunit_resource *);
>
> +void kunit_set_failure(struct kunit *test);
> +
> /**
> * struct kunit_resource - represents a *test managed resource*
> * @allocation: for the user to store arbitrary data.
> @@ -191,6 +193,9 @@ struct kunit {
> * protect it with some type of lock.
> */
> struct list_head resources; /* Protected by lock. */
> +
> + bool kasan_report_expected;
> + bool kasan_report_found;
> };
>
> void kunit_init_test(struct kunit *test, const char *name);
> @@ -941,6 +946,25 @@ do { \
> ptr, \
> NULL)
>
> +/**
> + * KUNIT_EXPECT_KASAN_FAIL() - Causes a test failure when the expression does
> + * not cause a KASAN error.
> + *
> + */
> +#define KUNIT_EXPECT_KASAN_FAIL(test, condition) do { \
> + test->kasan_report_expected = true; \
> + test->kasan_report_found = false; \
> + condition; \
> + if (test->kasan_report_found == test->kasan_report_expected) { \
> + pr_info("%d has passed", __LINE__); \
> + } else { \
> + kunit_set_failure(test); \
> + pr_info("%d has failed", __LINE__); \
> + } \
> + test->kasan_report_expected = false; \
> + test->kasan_report_found = false; \
> +} while (0)
> +
> /**
> * KUNIT_EXPECT_TRUE() - Causes a test failure when the expression is not true.
> * @test: The test context object.
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 04278493bf15..db23d56061e7 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -32,6 +32,8 @@
> #include <linux/posix-timers.h>
> #include <linux/rseq.h>
>
> +#include <kunit/test.h>
> +
> /* task_struct member predeclarations (sorted alphabetically): */
> struct audit_context;
> struct backing_dev_info;
> @@ -1178,7 +1180,10 @@ struct task_struct {
>
> #ifdef CONFIG_KASAN
> unsigned int kasan_depth;
> -#endif
> +#ifdef CONFIG_KUNIT
> + struct kunit *kasan_kunit_test;
> +#endif /* CONFIG_KUNIT */
> +#endif /* CONFIG_KASAN */
>
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> /* Index of current stored address in ret_stack: */
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 9242f932896c..d266b9495c67 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -9,11 +9,12 @@
> #include <kunit/test.h>
> #include <linux/kernel.h>
> #include <linux/sched/debug.h>
> +#include <linux/sched.h>
>
> #include "string-stream.h"
> #include "try-catch-impl.h"
>
> -static void kunit_set_failure(struct kunit *test)
> +void kunit_set_failure(struct kunit *test)
> {
> WRITE_ONCE(test->success, false);
> }
> @@ -236,6 +237,10 @@ static void kunit_try_run_case(void *data)
> struct kunit_suite *suite = ctx->suite;
> struct kunit_case *test_case = ctx->test_case;
>
> +#ifdef CONFIG_KASAN
> + current->kasan_kunit_test = test;
> +#endif
> +
> /*
> * kunit_run_case_internal may encounter a fatal error; if it does,
> * abort will be called, this thread will exit, and finally the parent
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 5ef9f24f566b..5554d23799a5 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -32,6 +32,8 @@
>
> #include <asm/sections.h>
>
> +#include <kunit/test.h>
> +
> #include "kasan.h"
> #include "../slab.h"
>
> @@ -461,6 +463,15 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
> u8 tag = get_tag(object);
>
> object = reset_tag(object);
> +
> + if (current->kasan_kunit_test) {
> + if (current->kasan_kunit_test->kasan_report_expected) {
> + current->kasan_kunit_test->kasan_report_found = true;
> + return;
I think we need to continue and print KASAN report even in this case.
2 reasons:
- tests don't check validity of printed reports, but at least human
can verify sanity of the report
- report printing code also needs to be tested, at least that it does
not crash/hang
If we don't print reports, it may look nicer, but will be less useful.
> + }
> + kunit_set_failure(current->kasan_kunit_test);
> + }
> +
next prev parent reply other threads:[~2020-03-04 6:35 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-27 2:43 [RFC PATCH 1/2] Port KASAN Tests to KUnit Patricia Alfonso
2020-02-27 2:43 ` [RFC PATCH 2/2] KUnit: KASAN Integration Patricia Alfonso
2020-02-27 14:04 ` Alan Maguire
2020-02-29 0:46 ` Patricia Alfonso
2020-03-03 16:40 ` Alan Maguire
2020-03-05 2:14 ` Patricia Alfonso
2020-03-05 7:46 ` Alan Maguire
2020-03-10 21:42 ` Brendan Higgins
2020-03-10 21:42 ` Brendan Higgins
2020-03-10 21:39 ` Brendan Higgins
2020-03-10 21:39 ` Brendan Higgins
2020-02-27 14:39 ` Dmitry Vyukov
2020-02-29 1:09 ` Patricia Alfonso
2020-03-01 6:26 ` Dmitry Vyukov
2020-02-27 14:43 ` Dmitry Vyukov
2020-02-29 1:23 ` Patricia Alfonso
2020-03-01 6:29 ` Dmitry Vyukov
2020-03-04 1:26 ` Patricia Alfonso
2020-03-04 6:23 ` Dmitry Vyukov
2020-03-05 0:07 ` Patricia Alfonso
2020-03-05 6:44 ` Dmitry Vyukov
2020-02-27 14:45 ` Dmitry Vyukov
2020-02-29 0:49 ` Patricia Alfonso
2020-03-04 6:35 ` Dmitry Vyukov [this message]
2020-02-27 13:56 ` [RFC PATCH 1/2] Port KASAN Tests to KUnit Andrey Konovalov
2020-02-27 15:16 ` Alan Maguire
2020-02-27 14:19 ` Dmitry Vyukov
2020-02-29 1:56 ` Patricia Alfonso
2020-03-01 6:39 ` Dmitry Vyukov
2020-03-02 17:52 ` Kees Cook
2020-03-02 22:36 ` Brendan Higgins
2020-03-02 23:02 ` Kees Cook
2020-03-06 23:58 ` Patricia Alfonso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACT4Y+avZ5ZdTUPQy7as3PAvdYd0NcSsyUukiumHa_Ah9ZYCFA@mail.gmail.com \
--to=dvyukov@google.com \
--cc=aryabinin@virtuozzo.com \
--cc=brendanhiggins@google.com \
--cc=davidgow@google.com \
--cc=juri.lelli@redhat.com \
--cc=kasan-dev@googlegroups.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=trishalfonso@google.com \
--cc=vincent.guittot@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).