Skip to content

Commit

Permalink
kasan: add compiler barriers to KUNIT_EXPECT_KASAN_FAIL
Browse files Browse the repository at this point in the history
It might not be obvious to the compiler that the expression must be
executed between writing and reading to fail_data. In this case, the
compiler might reorder or optimize away some of the accesses, and
the tests will fail.

Add compiler barriers around the expression in KUNIT_EXPECT_KASAN_FAIL
and use READ/WRITE_ONCE() for accessing fail_data fields.

Link: https://linux-review.googlesource.com/id/I046079f48641a1d36fe627fc8827a9249102fd50
Link: https://lkml.kernel.org/r/6f11596f367d8ae8f71d800351e9a5d91eda19f6.1610733117.git.andreyknvl@google.com
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Marco Elver <elver@google.com>
Reviewed-by: Alexander Potapenko <glider@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Branislav Rankov <Branislav.Rankov@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Evgenii Stepanov <eugenis@google.com>
Cc: Kevin Brodsky <kevin.brodsky@arm.com>
Cc: Peter Collingbourne <pcc@google.com>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
xairy authored and torvalds committed Feb 24, 2021
1 parent 5d92bdf commit 2e4bde6
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
17 changes: 12 additions & 5 deletions lib/test_kasan.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,30 @@ static void kasan_test_exit(struct kunit *test)
* normally auto-disabled. When this happens, this test handler reenables
* tag checking. As tag checking can be only disabled or enabled per CPU, this
* handler disables migration (preemption).
*
* Since the compiler doesn't see that the expression can change the fail_data
* 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.
*/
#define KUNIT_EXPECT_KASAN_FAIL(test, expression) do { \
if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) \
migrate_disable(); \
fail_data.report_expected = true; \
fail_data.report_found = false; \
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, \
fail_data.report_expected, \
fail_data.report_found); \
READ_ONCE(fail_data.report_expected), \
READ_ONCE(fail_data.report_found)); \
if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \
if (fail_data.report_found) \
if (READ_ONCE(fail_data.report_found)) \
kasan_enable_tagging(); \
migrate_enable(); \
} \
Expand Down
2 changes: 1 addition & 1 deletion mm/kasan/report.c
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ static void kasan_update_kunit_status(struct kunit *cur_test)
}

kasan_data = (struct kunit_kasan_expectation *)resource->data;
kasan_data->report_found = true;
WRITE_ONCE(kasan_data->report_found, true);
kunit_put_resource(resource);
}
#endif /* IS_ENABLED(CONFIG_KUNIT) */
Expand Down

0 comments on commit 2e4bde6

Please sign in to comment.