| Message ID | 48079c52cc329fbc52f4386996598d58022fb872.1617207873.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kasan: detect false-positives in tests | expand |
On Wed, 31 Mar 2021 at 18:25, Andrey Konovalov <andreyknvl@google.com> wrote: > > Currently, KASAN-KUnit tests can check that a particular annotated part > of code causes a KASAN report. However, they do not check that no unwanted > reports happen between the annotated parts. > > This patch implements these checks. > > It is done by setting report_data.report_found to false in > kasan_test_init() and at the end of KUNIT_EXPECT_KASAN_FAIL() and then > checking that it remains false at the beginning of > KUNIT_EXPECT_KASAN_FAIL() and in kasan_test_exit(). > > kunit_add_named_resource() call is moved to kasan_test_init(), and the > value of fail_data.report_expected is kept as false in between > KUNIT_EXPECT_KASAN_FAIL() annotations for consistency. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Reviewed-by: Marco Elver <elver@google.com> Thank you! > --- > lib/test_kasan.c | 49 +++++++++++++++++++++++++++--------------------- > 1 file changed, 28 insertions(+), 21 deletions(-) > > diff --git a/lib/test_kasan.c b/lib/test_kasan.c > index d77c45edc7cd..bf9225002a7e 100644 > --- a/lib/test_kasan.c > +++ b/lib/test_kasan.c > @@ -54,6 +54,10 @@ static int kasan_test_init(struct kunit *test) > > multishot = kasan_save_enable_multi_shot(); > kasan_set_tagging_report_once(false); > + fail_data.report_found = false; > + fail_data.report_expected = false; > + kunit_add_named_resource(test, NULL, NULL, &resource, > + "kasan_data", &fail_data); > return 0; > } > > @@ -61,6 +65,7 @@ static void kasan_test_exit(struct kunit *test) > { > kasan_set_tagging_report_once(true); > kasan_restore_multi_shot(multishot); > + KUNIT_EXPECT_FALSE(test, fail_data.report_found); > } > > /** > @@ -78,28 +83,30 @@ static void kasan_test_exit(struct kunit *test) > * fields, it can reorder or optimize away the accesses to those fields. > * Use READ/WRITE_ONCE() for the accesses and compiler barriers around the > * expression to prevent that. > + * > + * In between KUNIT_EXPECT_KASAN_FAIL checks, fail_data.report_found is kept as > + * false. This allows detecting KASAN reports that happen outside of the checks > + * by asserting !fail_data.report_found at the start of KUNIT_EXPECT_KASAN_FAIL > + * and in kasan_test_exit. > */ > -#define KUNIT_EXPECT_KASAN_FAIL(test, expression) do { \ > - if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) \ > - migrate_disable(); \ > - WRITE_ONCE(fail_data.report_expected, true); \ > - WRITE_ONCE(fail_data.report_found, false); \ > - kunit_add_named_resource(test, \ > - NULL, \ > - NULL, \ > - &resource, \ > - "kasan_data", &fail_data); \ > - barrier(); \ > - expression; \ > - barrier(); \ > - KUNIT_EXPECT_EQ(test, \ > - READ_ONCE(fail_data.report_expected), \ > - READ_ONCE(fail_data.report_found)); \ > - if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \ > - if (READ_ONCE(fail_data.report_found)) \ > - kasan_enable_tagging(); \ > - migrate_enable(); \ > - } \ > +#define KUNIT_EXPECT_KASAN_FAIL(test, expression) do { \ > + if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) \ > + migrate_disable(); \ > + KUNIT_EXPECT_FALSE(test, READ_ONCE(fail_data.report_found)); \ > + WRITE_ONCE(fail_data.report_expected, true); \ > + barrier(); \ > + expression; \ > + barrier(); \ > + KUNIT_EXPECT_EQ(test, \ > + READ_ONCE(fail_data.report_expected), \ > + READ_ONCE(fail_data.report_found)); \ > + if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \ > + if (READ_ONCE(fail_data.report_found)) \ > + kasan_enable_tagging(); \ > + migrate_enable(); \ > + } \ > + WRITE_ONCE(fail_data.report_found, false); \ > + WRITE_ONCE(fail_data.report_expected, false); \ > } while (0) > > #define KASAN_TEST_NEEDS_CONFIG_ON(test, config) do { \ > -- > 2.31.0.291.g576ba9dcdaf-goog >
diff --git a/lib/test_kasan.c b/lib/test_kasan.c index d77c45edc7cd..bf9225002a7e 100644 --- a/lib/test_kasan.c +++ b/lib/test_kasan.c @@ -54,6 +54,10 @@ static int kasan_test_init(struct kunit *test) multishot = kasan_save_enable_multi_shot(); kasan_set_tagging_report_once(false); + fail_data.report_found = false; + fail_data.report_expected = false; + kunit_add_named_resource(test, NULL, NULL, &resource, + "kasan_data", &fail_data); return 0; } @@ -61,6 +65,7 @@ static void kasan_test_exit(struct kunit *test) { kasan_set_tagging_report_once(true); kasan_restore_multi_shot(multishot); + KUNIT_EXPECT_FALSE(test, fail_data.report_found); } /** @@ -78,28 +83,30 @@ static void kasan_test_exit(struct kunit *test) * fields, it can reorder or optimize away the accesses to those fields. * Use READ/WRITE_ONCE() for the accesses and compiler barriers around the * expression to prevent that. + * + * In between KUNIT_EXPECT_KASAN_FAIL checks, fail_data.report_found is kept as + * false. This allows detecting KASAN reports that happen outside of the checks + * by asserting !fail_data.report_found at the start of KUNIT_EXPECT_KASAN_FAIL + * and in kasan_test_exit. */ -#define KUNIT_EXPECT_KASAN_FAIL(test, expression) do { \ - if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) \ - migrate_disable(); \ - WRITE_ONCE(fail_data.report_expected, true); \ - WRITE_ONCE(fail_data.report_found, false); \ - kunit_add_named_resource(test, \ - NULL, \ - NULL, \ - &resource, \ - "kasan_data", &fail_data); \ - barrier(); \ - expression; \ - barrier(); \ - KUNIT_EXPECT_EQ(test, \ - READ_ONCE(fail_data.report_expected), \ - READ_ONCE(fail_data.report_found)); \ - if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \ - if (READ_ONCE(fail_data.report_found)) \ - kasan_enable_tagging(); \ - migrate_enable(); \ - } \ +#define KUNIT_EXPECT_KASAN_FAIL(test, expression) do { \ + if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) \ + migrate_disable(); \ + KUNIT_EXPECT_FALSE(test, READ_ONCE(fail_data.report_found)); \ + WRITE_ONCE(fail_data.report_expected, true); \ + barrier(); \ + expression; \ + barrier(); \ + KUNIT_EXPECT_EQ(test, \ + READ_ONCE(fail_data.report_expected), \ + READ_ONCE(fail_data.report_found)); \ + if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \ + if (READ_ONCE(fail_data.report_found)) \ + kasan_enable_tagging(); \ + migrate_enable(); \ + } \ + WRITE_ONCE(fail_data.report_found, false); \ + WRITE_ONCE(fail_data.report_expected, false); \ } while (0) #define KASAN_TEST_NEEDS_CONFIG_ON(test, config) do { \
Currently, KASAN-KUnit tests can check that a particular annotated part of code causes a KASAN report. However, they do not check that no unwanted reports happen between the annotated parts. This patch implements these checks. It is done by setting report_data.report_found to false in kasan_test_init() and at the end of KUNIT_EXPECT_KASAN_FAIL() and then checking that it remains false at the beginning of KUNIT_EXPECT_KASAN_FAIL() and in kasan_test_exit(). kunit_add_named_resource() call is moved to kasan_test_init(), and the value of fail_data.report_expected is kept as false in between KUNIT_EXPECT_KASAN_FAIL() annotations for consistency. Signed-off-by: Andrey Konovalov <andreyknvl@google.com> --- lib/test_kasan.c | 49 +++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-)