| Message ID | 20210115120043.50023-2-vincenzo.frascino@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: ARMv8.5-A: MTE: Add async mode support | expand |
On Fri, Jan 15, 2021 at 12:00:40PM +0000, Vincenzo Frascino wrote: > Architectures supported by KASAN HW can provide a light mode of > execution. On an MTE enabled arm64 hw for example this can be identified > with the asynch mode of execution. > In this mode, if a tag check fault occurs, the TFSR_EL1 register is > updated asynchronously. The kernel checks the corresponding bits > periodically. What's the expected usage of this relative to prod, given that this has to be chosen at boot time? When/where is this expected to be used relative to prod mode? > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h > index 18fce223b67b..3a7c5beb7096 100644 > --- a/arch/arm64/include/asm/memory.h > +++ b/arch/arm64/include/asm/memory.h > @@ -231,7 +231,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_enable_tagging(mode) mte_enable_kernel(mode) Rather than passing a mode in, I think it'd be better to have: * arch_enable_tagging_prod() * arch_enable_tagging_light() ... that we can map in the arch code to separate: * mte_enable_kernel_sync() * mte_enable_kernel_async() ... as by construction that avoids calls with an unhandled mode, and we wouldn't need the mode enum kasan_hw_tags_mode... > +static inline int hw_init_mode(enum kasan_arg_mode mode) > +{ > + switch (mode) { > + case KASAN_ARG_MODE_LIGHT: > + return KASAN_HW_TAGS_ASYNC; > + default: > + return KASAN_HW_TAGS_SYNC; > + } > +} ... and we can just have a wrapper like this to call either of the two functions directly, i.e. static inline void hw_enable_tagging_mode(enum kasan_arg_mode mode) { if (mode == KASAN_ARG_MODE_LIGHT) arch_enable_tagging_mode_light(); else arch_enable_tagging_mode_prod(); } Thanks, Mark.
On Fri, Jan 15, 2021 at 1:00 PM Vincenzo Frascino <vincenzo.frascino@arm.com> wrote: > > Architectures supported by KASAN HW can provide a light mode of > execution. On an MTE enabled arm64 hw for example this can be identified > with the asynch mode of execution. > In this mode, if a tag check fault occurs, the TFSR_EL1 register is > updated asynchronously. The kernel checks the corresponding bits > periodically. > > KASAN requires a specific mode of execution to make use of this hw feature. > > Add KASAN HW light execution mode. > > Note: This patch adds the KASAN_ARG_MODE_LIGHT config option and the > "light" kernel command line option to enable the described feature. > This patch introduces the kasan_def.h header to make easier to propagate > the relevant enumerations to the architectural code. > > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> > Cc: Alexander Potapenko <glider@google.com> > Cc: Andrey Konovalov <andreyknvl@google.com> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> > --- > arch/arm64/include/asm/memory.h | 2 +- > arch/arm64/include/asm/mte-kasan.h | 5 +++-- > arch/arm64/kernel/mte.c | 2 +- > include/linux/kasan.h | 1 + > include/linux/kasan_def.h | 10 ++++++++++ > mm/kasan/hw_tags.c | 19 ++++++++++++++++++- > mm/kasan/kasan.h | 2 +- > 7 files changed, 35 insertions(+), 6 deletions(-) > create mode 100644 include/linux/kasan_def.h > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h > index 18fce223b67b..3a7c5beb7096 100644 > --- a/arch/arm64/include/asm/memory.h > +++ b/arch/arm64/include/asm/memory.h > @@ -231,7 +231,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_enable_tagging(mode) mte_enable_kernel(mode) > #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..5402f4c8e88d 100644 > --- a/arch/arm64/include/asm/mte-kasan.h > +++ b/arch/arm64/include/asm/mte-kasan.h > @@ -9,6 +9,7 @@ > > #ifndef __ASSEMBLY__ > > +#include <linux/kasan_def.h> > #include <linux/types.h> > > /* > @@ -29,7 +30,7 @@ u8 mte_get_mem_tag(void *addr); > u8 mte_get_random_tag(void); > void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag); > > -void mte_enable_kernel(void); > +void mte_enable_kernel(enum kasan_hw_tags_mode mode); > void mte_init_tags(u64 max_tag); > > #else /* CONFIG_ARM64_MTE */ > @@ -52,7 +53,7 @@ static inline void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag) > return addr; > } > > -static inline void mte_enable_kernel(void) > +static inline void mte_enable_kernel(enum kasan_hw_tags_mode mode) > { > } > > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c > index dc9ada64feed..53a6d734e29b 100644 > --- a/arch/arm64/kernel/mte.c > +++ b/arch/arm64/kernel/mte.c > @@ -151,7 +151,7 @@ void mte_init_tags(u64 max_tag) > write_sysreg_s(SYS_GCR_EL1_RRND | gcr_kernel_excl, SYS_GCR_EL1); > } > > -void mte_enable_kernel(void) > +void mte_enable_kernel(enum kasan_hw_tags_mode mode) > { > /* Enable MTE Sync Mode for EL1. */ > sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC); > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > index 5e0655fb2a6f..026031444217 100644 > --- a/include/linux/kasan.h > +++ b/include/linux/kasan.h > @@ -2,6 +2,7 @@ > #ifndef _LINUX_KASAN_H > #define _LINUX_KASAN_H > > +#include <linux/kasan_def.h> > #include <linux/static_key.h> > #include <linux/types.h> > > diff --git a/include/linux/kasan_def.h b/include/linux/kasan_def.h > new file mode 100644 > index 000000000000..0a55400809c9 > --- /dev/null > +++ b/include/linux/kasan_def.h > @@ -0,0 +1,10 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_KASAN_DEF_H > +#define _LINUX_KASAN_DEF_H > + > +enum kasan_hw_tags_mode { > + KASAN_HW_TAGS_SYNC, > + KASAN_HW_TAGS_ASYNC, > +}; > + > +#endif /* _LINUX_KASAN_DEF_H */ > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c > index 55bd6f09c70f..6c3b0742f639 100644 > --- a/mm/kasan/hw_tags.c > +++ b/mm/kasan/hw_tags.c > @@ -22,6 +22,7 @@ > enum kasan_arg_mode { > KASAN_ARG_MODE_DEFAULT, > KASAN_ARG_MODE_OFF, > + KASAN_ARG_MODE_LIGHT, > KASAN_ARG_MODE_PROD, > KASAN_ARG_MODE_FULL, > }; > @@ -60,6 +61,8 @@ static int __init early_kasan_mode(char *arg) > > if (!strcmp(arg, "off")) > kasan_arg_mode = KASAN_ARG_MODE_OFF; > + else if (!strcmp(arg, "light")) > + kasan_arg_mode = KASAN_ARG_MODE_LIGHT; Hi Vincenzo, I've just mailed the change to KASAN parameters [1] as discussed, so we should use a standalone parameter here (kasan.trap?). Thanks! [1] https://lkml.org/lkml/2021/1/15/1242
Hi Andrey, On 1/15/21 6:59 PM, Andrey Konovalov wrote: > On Fri, Jan 15, 2021 at 1:00 PM Vincenzo Frascino > <vincenzo.frascino@arm.com> wrote: >> [...] >> @@ -60,6 +61,8 @@ static int __init early_kasan_mode(char *arg) >> >> if (!strcmp(arg, "off")) >> kasan_arg_mode = KASAN_ARG_MODE_OFF; >> + else if (!strcmp(arg, "light")) >> + kasan_arg_mode = KASAN_ARG_MODE_LIGHT; > > Hi Vincenzo, > > I've just mailed the change to KASAN parameters [1] as discussed, so > we should use a standalone parameter here (kasan.trap?). > > Thanks! > > [1] https://lkml.org/lkml/2021/1/15/1242 > Thanks for this. I will have a look into it today. In the meantime, could you please elaborate a bit more on kasan.trap?
Hi Mark, On 1/15/21 3:08 PM, Mark Rutland wrote: > On Fri, Jan 15, 2021 at 12:00:40PM +0000, Vincenzo Frascino wrote: >> Architectures supported by KASAN HW can provide a light mode of >> execution. On an MTE enabled arm64 hw for example this can be identified >> with the asynch mode of execution. >> In this mode, if a tag check fault occurs, the TFSR_EL1 register is >> updated asynchronously. The kernel checks the corresponding bits >> periodically. > > What's the expected usage of this relative to prod, given that this has > to be chosen at boot time? When/where is this expected to be used > relative to prod mode? > IIUC the light mode is meant for low spec devices. I let Andrey comment a bit more on this topic. >> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h >> index 18fce223b67b..3a7c5beb7096 100644 >> --- a/arch/arm64/include/asm/memory.h >> +++ b/arch/arm64/include/asm/memory.h >> @@ -231,7 +231,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_enable_tagging(mode) mte_enable_kernel(mode) > > Rather than passing a mode in, I think it'd be better to have: > > * arch_enable_tagging_prod() > * arch_enable_tagging_light() > > ... that we can map in the arch code to separate: > > * mte_enable_kernel_sync() > * mte_enable_kernel_async() > > ... as by construction that avoids calls with an unhandled mode, and we > wouldn't need the mode enum kasan_hw_tags_mode... > >> +static inline int hw_init_mode(enum kasan_arg_mode mode) >> +{ >> + switch (mode) { >> + case KASAN_ARG_MODE_LIGHT: >> + return KASAN_HW_TAGS_ASYNC; >> + default: >> + return KASAN_HW_TAGS_SYNC; >> + } >> +} > > ... and we can just have a wrapper like this to call either of the two functions directly, i.e. > > static inline void hw_enable_tagging_mode(enum kasan_arg_mode mode) > { > if (mode == KASAN_ARG_MODE_LIGHT) > arch_enable_tagging_mode_light(); > else > arch_enable_tagging_mode_prod(); > } > Fine by me, this would remove the need of adding a new enumeration as well and reflect on the arch code. I would keep "arch_enable_tagging_mode_sync" and "arch_enable_tagging_mode_async" though to give a clear indication in the KASAN code of the mode we are setting. I will adapt my code accordingly for v4. > Thanks, > Mark. >
On Sat, Jan 16, 2021 at 2:37 PM Vincenzo Frascino <vincenzo.frascino@arm.com> wrote: > > > [1] https://lkml.org/lkml/2021/1/15/1242 > > > > Thanks for this. I will have a look into it today. In the meantime, could you > please elaborate a bit more on kasan.trap? That's what I call the boot parameter that allows switching between sync and async. We'll need one as we're dropping kasan.mode=off/prod/light/full. Feel free to name it differently. Perhaps, as kasan.mode is now unused, we can use that for sync/async.
On 1/16/21 1:59 PM, Andrey Konovalov wrote: > On Sat, Jan 16, 2021 at 2:37 PM Vincenzo Frascino > <vincenzo.frascino@arm.com> wrote: >> >>> [1] https://lkml.org/lkml/2021/1/15/1242 >>> >> >> Thanks for this. I will have a look into it today. In the meantime, could you >> please elaborate a bit more on kasan.trap? > > That's what I call the boot parameter that allows switching between > sync and async. We'll need one as we're dropping > kasan.mode=off/prod/light/full. > > Feel free to name it differently. Perhaps, as kasan.mode is now > unused, we can use that for sync/async. > I see, thanks for the explanation. "mode" or "trap" would work for me.
On Sat, Jan 16, 2021 at 2:43 PM Vincenzo Frascino <vincenzo.frascino@arm.com> wrote: > On 1/15/21 3:08 PM, Mark Rutland wrote: > > On Fri, Jan 15, 2021 at 12:00:40PM +0000, Vincenzo Frascino wrote: > >> Architectures supported by KASAN HW can provide a light mode of > >> execution. On an MTE enabled arm64 hw for example this can be identified > >> with the asynch mode of execution. > >> In this mode, if a tag check fault occurs, the TFSR_EL1 register is > >> updated asynchronously. The kernel checks the corresponding bits > >> periodically. > > > > What's the expected usage of this relative to prod, given that this has > > to be chosen at boot time? When/where is this expected to be used > > relative to prod mode? Hi Mark, Sync + no panic (what is called prod right now) + logging is for the initial MTE integration stage as causing panics is risky. There's no way to know how often MTE-detected bugs will happen during normal usage as the kernel is buggy. Eventually, we're hoping to switch to sync + panic to allow MTE to act as a security mitigation. For devices where the slowdown caused by sync is untolerable, there'll be an option to use async, which is significantly faster. The exact perf numbers are yet to be measured properly, I'll share them with one of the future patches. Thanks!
On Sat, Jan 16, 2021 at 01:47:08PM +0000, Vincenzo Frascino wrote: > On 1/15/21 3:08 PM, Mark Rutland wrote: > > On Fri, Jan 15, 2021 at 12:00:40PM +0000, Vincenzo Frascino wrote: > >> #ifdef CONFIG_KASAN_HW_TAGS > >> -#define arch_enable_tagging() mte_enable_kernel() > >> +#define arch_enable_tagging(mode) mte_enable_kernel(mode) > > > > Rather than passing a mode in, I think it'd be better to have: > > > > * arch_enable_tagging_prod() > > * arch_enable_tagging_light() > > > > ... that we can map in the arch code to separate: > > > > * mte_enable_kernel_sync() > > * mte_enable_kernel_async() > > > > ... as by construction that avoids calls with an unhandled mode, and we > > wouldn't need the mode enum kasan_hw_tags_mode... > > > >> +static inline int hw_init_mode(enum kasan_arg_mode mode) > >> +{ > >> + switch (mode) { > >> + case KASAN_ARG_MODE_LIGHT: > >> + return KASAN_HW_TAGS_ASYNC; > >> + default: > >> + return KASAN_HW_TAGS_SYNC; > >> + } > >> +} > > > > ... and we can just have a wrapper like this to call either of the two functions directly, i.e. > > > > static inline void hw_enable_tagging_mode(enum kasan_arg_mode mode) > > { > > if (mode == KASAN_ARG_MODE_LIGHT) > > arch_enable_tagging_mode_light(); > > else > > arch_enable_tagging_mode_prod(); > > } > > > > Fine by me, this would remove the need of adding a new enumeration as well and > reflect on the arch code. I would keep "arch_enable_tagging_mode_sync" and > "arch_enable_tagging_mode_async" though to give a clear indication in the KASAN > code of the mode we are setting. I will adapt my code accordingly for v4. Thanks, that sounds great! I completely agree on keeping the '_sync' and '_aync' suffixes in the the core code. Mark.
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h index 18fce223b67b..3a7c5beb7096 100644 --- a/arch/arm64/include/asm/memory.h +++ b/arch/arm64/include/asm/memory.h @@ -231,7 +231,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_enable_tagging(mode) mte_enable_kernel(mode) #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..5402f4c8e88d 100644 --- a/arch/arm64/include/asm/mte-kasan.h +++ b/arch/arm64/include/asm/mte-kasan.h @@ -9,6 +9,7 @@ #ifndef __ASSEMBLY__ +#include <linux/kasan_def.h> #include <linux/types.h> /* @@ -29,7 +30,7 @@ u8 mte_get_mem_tag(void *addr); u8 mte_get_random_tag(void); void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag); -void mte_enable_kernel(void); +void mte_enable_kernel(enum kasan_hw_tags_mode mode); void mte_init_tags(u64 max_tag); #else /* CONFIG_ARM64_MTE */ @@ -52,7 +53,7 @@ static inline void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag) return addr; } -static inline void mte_enable_kernel(void) +static inline void mte_enable_kernel(enum kasan_hw_tags_mode mode) { } diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c index dc9ada64feed..53a6d734e29b 100644 --- a/arch/arm64/kernel/mte.c +++ b/arch/arm64/kernel/mte.c @@ -151,7 +151,7 @@ void mte_init_tags(u64 max_tag) write_sysreg_s(SYS_GCR_EL1_RRND | gcr_kernel_excl, SYS_GCR_EL1); } -void mte_enable_kernel(void) +void mte_enable_kernel(enum kasan_hw_tags_mode mode) { /* Enable MTE Sync Mode for EL1. */ sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC); diff --git a/include/linux/kasan.h b/include/linux/kasan.h index 5e0655fb2a6f..026031444217 100644 --- a/include/linux/kasan.h +++ b/include/linux/kasan.h @@ -2,6 +2,7 @@ #ifndef _LINUX_KASAN_H #define _LINUX_KASAN_H +#include <linux/kasan_def.h> #include <linux/static_key.h> #include <linux/types.h> diff --git a/include/linux/kasan_def.h b/include/linux/kasan_def.h new file mode 100644 index 000000000000..0a55400809c9 --- /dev/null +++ b/include/linux/kasan_def.h @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_KASAN_DEF_H +#define _LINUX_KASAN_DEF_H + +enum kasan_hw_tags_mode { + KASAN_HW_TAGS_SYNC, + KASAN_HW_TAGS_ASYNC, +}; + +#endif /* _LINUX_KASAN_DEF_H */ diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c index 55bd6f09c70f..6c3b0742f639 100644 --- a/mm/kasan/hw_tags.c +++ b/mm/kasan/hw_tags.c @@ -22,6 +22,7 @@ enum kasan_arg_mode { KASAN_ARG_MODE_DEFAULT, KASAN_ARG_MODE_OFF, + KASAN_ARG_MODE_LIGHT, KASAN_ARG_MODE_PROD, KASAN_ARG_MODE_FULL, }; @@ -60,6 +61,8 @@ static int __init early_kasan_mode(char *arg) if (!strcmp(arg, "off")) kasan_arg_mode = KASAN_ARG_MODE_OFF; + else if (!strcmp(arg, "light")) + kasan_arg_mode = KASAN_ARG_MODE_LIGHT; else if (!strcmp(arg, "prod")) kasan_arg_mode = KASAN_ARG_MODE_PROD; else if (!strcmp(arg, "full")) @@ -105,9 +108,21 @@ static int __init early_kasan_fault(char *arg) } early_param("kasan.fault", early_kasan_fault); +static inline int hw_init_mode(enum kasan_arg_mode mode) +{ + switch (mode) { + case KASAN_ARG_MODE_LIGHT: + return KASAN_HW_TAGS_ASYNC; + default: + return KASAN_HW_TAGS_SYNC; + } +} + /* kasan_init_hw_tags_cpu() is called for each CPU. */ void kasan_init_hw_tags_cpu(void) { + enum kasan_hw_tags_mode hw_mode; + /* * There's no need to check that the hardware is MTE-capable here, * as this function is only called for MTE-capable hardware. @@ -118,7 +133,8 @@ void kasan_init_hw_tags_cpu(void) return; hw_init_tags(KASAN_TAG_MAX); - hw_enable_tagging(); + hw_mode = hw_init_mode(kasan_arg_mode); + hw_enable_tagging(hw_mode); } /* kasan_init_hw_tags() is called once on boot CPU. */ @@ -145,6 +161,7 @@ void __init kasan_init_hw_tags(void) case KASAN_ARG_MODE_OFF: /* If KASAN is disabled, do nothing. */ return; + case KASAN_ARG_MODE_LIGHT: case KASAN_ARG_MODE_PROD: static_branch_enable(&kasan_flag_enabled); break; diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h index cc4d9e1d49b1..78c09279327e 100644 --- a/mm/kasan/kasan.h +++ b/mm/kasan/kasan.h @@ -284,7 +284,7 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag) #define arch_set_mem_tag_range(addr, size, tag) ((void *)(addr)) #endif -#define hw_enable_tagging() arch_enable_tagging() +#define hw_enable_tagging(mode) arch_enable_tagging(mode) #define hw_init_tags(max_tag) arch_init_tags(max_tag) #define hw_get_random_tag() arch_get_random_tag() #define hw_get_mem_tag(addr) arch_get_mem_tag(addr)
Architectures supported by KASAN HW can provide a light mode of execution. On an MTE enabled arm64 hw for example this can be identified with the asynch mode of execution. In this mode, if a tag check fault occurs, the TFSR_EL1 register is updated asynchronously. The kernel checks the corresponding bits periodically. KASAN requires a specific mode of execution to make use of this hw feature. Add KASAN HW light execution mode. Note: This patch adds the KASAN_ARG_MODE_LIGHT config option and the "light" kernel command line option to enable the described feature. This patch introduces the kasan_def.h header to make easier to propagate the relevant enumerations to the architectural code. Cc: Dmitry Vyukov <dvyukov@google.com> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> Cc: Alexander Potapenko <glider@google.com> Cc: Andrey Konovalov <andreyknvl@google.com> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> --- arch/arm64/include/asm/memory.h | 2 +- arch/arm64/include/asm/mte-kasan.h | 5 +++-- arch/arm64/kernel/mte.c | 2 +- include/linux/kasan.h | 1 + include/linux/kasan_def.h | 10 ++++++++++ mm/kasan/hw_tags.c | 19 ++++++++++++++++++- mm/kasan/kasan.h | 2 +- 7 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 include/linux/kasan_def.h