Hi Andrey, On 1/5/21 6:27 PM, Andrey Konovalov wrote: > On a high level, this patch allows running KUnit KASAN tests with the > hardware tag-based KASAN mode. > > Internally, this change reenables tag checking at the end of each KASAN > test that triggers a tag fault and leads to tag checking being disabled. > > With this patch KASAN tests are still failing for the hardware tag-based > mode; fixes come in the next few patches. >
A part what Catalin noticed already: Reviewed-by: Vincenzo Frascino <vincenzo.frasc...@arm.com> > Signed-off-by: Andrey Konovalov <andreyk...@google.com> > Link: > https://linux-review.googlesource.com/id/Id94dc9eccd33b23cda4950be408c27f879e474c8 > --- > arch/arm64/include/asm/memory.h | 1 + > arch/arm64/include/asm/mte-kasan.h | 12 +++++++++ > arch/arm64/kernel/mte.c | 12 +++++++++ > arch/arm64/mm/fault.c | 16 +++++++----- > lib/Kconfig.kasan | 4 +-- > lib/test_kasan.c | 42 +++++++++++++++++++++--------- > mm/kasan/kasan.h | 9 +++++++ > 7 files changed, 75 insertions(+), 21 deletions(-) > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h > index 18fce223b67b..cedfc9e97bcc 100644 > --- a/arch/arm64/include/asm/memory.h > +++ b/arch/arm64/include/asm/memory.h > @@ -232,6 +232,7 @@ static inline const void *__tag_set(const void *addr, u8 > tag) > > #ifdef CONFIG_KASAN_HW_TAGS > #define arch_enable_tagging() mte_enable_kernel() > +#define arch_set_tagging_report_once(state) mte_set_report_once(state) > #define arch_init_tags(max_tag) mte_init_tags(max_tag) > #define arch_get_random_tag() mte_get_random_tag() > #define arch_get_mem_tag(addr) mte_get_mem_tag(addr) > diff --git a/arch/arm64/include/asm/mte-kasan.h > b/arch/arm64/include/asm/mte-kasan.h > index 26349a4b5e2e..3748d5bb88c0 100644 > --- a/arch/arm64/include/asm/mte-kasan.h > +++ b/arch/arm64/include/asm/mte-kasan.h > @@ -32,6 +32,9 @@ void *mte_set_mem_tag_range(void *addr, size_t size, u8 > tag); > void mte_enable_kernel(void); > void mte_init_tags(u64 max_tag); > > +void mte_set_report_once(bool state); > +bool mte_report_once(void); > + > #else /* CONFIG_ARM64_MTE */ > > static inline u8 mte_get_ptr_tag(void *ptr) > @@ -60,6 +63,15 @@ static inline void mte_init_tags(u64 max_tag) > { > } > > +static inline void mte_set_report_once(bool state) > +{ > +} > + > +static inline bool mte_report_once(void) > +{ > + return false; > +} > + > #endif /* CONFIG_ARM64_MTE */ > > #endif /* __ASSEMBLY__ */ > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c > index dc9ada64feed..c63b3d7a3cd9 100644 > --- a/arch/arm64/kernel/mte.c > +++ b/arch/arm64/kernel/mte.c > @@ -25,6 +25,8 @@ > > u64 gcr_kernel_excl __ro_after_init; > > +static bool report_fault_once = true; > + > static void mte_sync_page_tags(struct page *page, pte_t *ptep, bool > check_swap) > { > pte_t old_pte = READ_ONCE(*ptep); > @@ -158,6 +160,16 @@ void mte_enable_kernel(void) > isb(); > } > > +void mte_set_report_once(bool state) > +{ > + WRITE_ONCE(report_fault_once, state); > +} > + > +bool mte_report_once(void) > +{ > + return READ_ONCE(report_fault_once); > +} > + > static void update_sctlr_el1_tcf0(u64 tcf0) > { > /* ISB required for the kernel uaccess routines */ > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index 3c40da479899..57d3f165d907 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -302,12 +302,20 @@ static void die_kernel_fault(const char *msg, unsigned > long addr, > static void report_tag_fault(unsigned long addr, unsigned int esr, > struct pt_regs *regs) > { > - bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0; > + static bool reported; > + bool is_write; > + > + if (READ_ONCE(reported)) > + return; > + > + if (mte_report_once()) > + WRITE_ONCE(reported, true); > > /* > * SAS bits aren't set for all faults reported in EL1, so we can't > * find out access size. > */ > + is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0; > kasan_report(addr, 0, is_write, regs->pc); > } > #else > @@ -319,12 +327,8 @@ static inline void report_tag_fault(unsigned long addr, > unsigned int esr, > static void do_tag_recovery(unsigned long addr, unsigned int esr, > struct pt_regs *regs) > { > - static bool reported; > > - if (!READ_ONCE(reported)) { > - report_tag_fault(addr, esr, regs); > - WRITE_ONCE(reported, true); > - } > + report_tag_fault(addr, esr, regs); > > /* > * Disable MTE Tag Checking on the local CPU for the current EL. > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > index f5fa4ba126bf..3091432acb0a 100644 > --- a/lib/Kconfig.kasan > +++ b/lib/Kconfig.kasan > @@ -190,11 +190,11 @@ config KASAN_KUNIT_TEST > kernel debugging features like KASAN. > > For more information on KUnit and unit tests in general, please refer > - to the KUnit documentation in Documentation/dev-tools/kunit > + to the KUnit documentation in Documentation/dev-tools/kunit. > > config TEST_KASAN_MODULE > tristate "KUnit-incompatible tests of KASAN bug detection capabilities" > - depends on m && KASAN > + depends on m && KASAN && !KASAN_HW_TAGS > help > This is a part of the KASAN test suite that is incompatible with > KUnit. Currently includes tests that do bad copy_from/to_user > diff --git a/lib/test_kasan.c b/lib/test_kasan.c > index f1eda0bcc780..dd3d2f95c24e 100644 > --- a/lib/test_kasan.c > +++ b/lib/test_kasan.c > @@ -41,16 +41,20 @@ static bool multishot; > > /* > * Temporarily enable multi-shot mode. Otherwise, KASAN would only report the > - * first detected bug and panic the kernel if panic_on_warn is enabled. > + * first detected bug and panic the kernel if panic_on_warn is enabled. For > + * hardware tag-based KASAN also allow tag checking to be reenabled for each > + * test, see the comment for KUNIT_EXPECT_KASAN_FAIL(). > */ > static int kasan_test_init(struct kunit *test) > { > multishot = kasan_save_enable_multi_shot(); > + hw_set_tagging_report_once(false); > return 0; > } > > static void kasan_test_exit(struct kunit *test) > { > + hw_set_tagging_report_once(true); > kasan_restore_multi_shot(multishot); > } > > @@ -59,19 +63,31 @@ static void kasan_test_exit(struct kunit *test) > * KASAN report; causes a test failure otherwise. This relies on a KUnit > * resource named "kasan_data". Do not use this name for KUnit resources > * outside of KASAN tests. > + * > + * For hardware tag-based KASAN, when a tag fault happens, tag checking is > + * 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). > */ > -#define KUNIT_EXPECT_KASAN_FAIL(test, expression) do { \ > - fail_data.report_expected = true; \ > - fail_data.report_found = false; \ > - kunit_add_named_resource(test, \ > - NULL, \ > - NULL, \ > - &resource, \ > - "kasan_data", &fail_data); \ > - expression; \ > - KUNIT_EXPECT_EQ(test, \ > - fail_data.report_expected, \ > - fail_data.report_found); \ > +#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; \ > + kunit_add_named_resource(test, \ > + NULL, \ > + NULL, \ > + &resource, \ > + "kasan_data", &fail_data); \ > + expression; \ > + KUNIT_EXPECT_EQ(test, \ > + fail_data.report_expected, \ > + fail_data.report_found); \ > + if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \ > + if (fail_data.report_found) \ > + hw_enable_tagging(); \ > + migrate_enable(); \ > + } \ > } while (0) > > static void kmalloc_oob_right(struct kunit *test) > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > index c3fb9bf241d3..292dfbc37deb 100644 > --- a/mm/kasan/kasan.h > +++ b/mm/kasan/kasan.h > @@ -280,6 +280,9 @@ static inline const void *arch_kasan_set_tag(const void > *addr, u8 tag) > #ifndef arch_init_tags > #define arch_init_tags(max_tag) > #endif > +#ifndef arch_set_tagging_report_once > +#define arch_set_tagging_report_once(state) > +#endif > #ifndef arch_get_random_tag > #define arch_get_random_tag() (0xFF) > #endif > @@ -292,10 +295,16 @@ static inline const void *arch_kasan_set_tag(const void > *addr, u8 tag) > > #define hw_enable_tagging() arch_enable_tagging() > #define hw_init_tags(max_tag) arch_init_tags(max_tag) > +#define hw_set_tagging_report_once(state) > arch_set_tagging_report_once(state) > #define hw_get_random_tag() arch_get_random_tag() > #define hw_get_mem_tag(addr) arch_get_mem_tag(addr) > #define hw_set_mem_tag_range(addr, size, tag) > arch_set_mem_tag_range((addr), (size), (tag)) > > +#else /* CONFIG_KASAN_HW_TAGS */ > + > +#define hw_enable_tagging() > +#define hw_set_tagging_report_once(state) > + > #endif /* CONFIG_KASAN_HW_TAGS */ > > #ifdef CONFIG_KASAN_SW_TAGS > -- Regards, Vincenzo
