| Message ID | fdf9e3aec8f57ebb2795710195f8aaf79e3b45bd.1605046662.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kasan: boot parameters for hardware tag-based mode | expand |
On Tue, Nov 10, 2020 at 11:20PM +0100, 'Andrey Konovalov' via kasan-dev wrote: > Hardware tag-based KASAN mode is intended to eventually be used in > production as a security mitigation. Therefore there's a need for finer > control over KASAN features and for an existence of a kill switch. > > This change adds a few boot parameters for hardware tag-based KASAN that > allow to disable or otherwise control particular KASAN features. > > The features that can be controlled are: > > 1. Whether KASAN is enabled at all. > 2. Whether KASAN collects and saves alloc/free stacks. > 3. Whether KASAN panics on a detected bug or not. > > With this change a new boot parameter kasan.mode allows to choose one of > three main modes: > > - kasan.mode=off - KASAN is disabled, no tag checks are performed > - kasan.mode=prod - only essential production features are enabled > - kasan.mode=full - all KASAN features are enabled > > The chosen mode provides default control values for the features mentioned > above. However it's also possible to override the default values by > providing: > > - kasan.stacktrace=off/on - enable alloc/free stack collection > (default: on for mode=full, otherwise off) > - kasan.fault=report/panic - only report tag fault or also panic > (default: report) > > If kasan.mode parameter is not provided, it defaults to full when > CONFIG_DEBUG_KERNEL is enabled, and to prod otherwise. > > It is essential that switching between these modes doesn't require > rebuilding the kernel with different configs, as this is required by > the Android GKI (Generic Kernel Image) initiative [1]. > > [1] https://source.android.com/devices/architecture/kernel/generic-kernel-image > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > Link: https://linux-review.googlesource.com/id/If7d37003875b2ed3e0935702c8015c223d6416a4 > --- > mm/kasan/common.c | 22 +++++-- > mm/kasan/hw_tags.c | 152 +++++++++++++++++++++++++++++++++++++++++++++ > mm/kasan/kasan.h | 16 +++++ > mm/kasan/report.c | 14 ++++- > 4 files changed, 197 insertions(+), 7 deletions(-) > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > index 4598c1364f19..efad5ed6a3bd 100644 > --- a/mm/kasan/common.c > +++ b/mm/kasan/common.c > @@ -129,6 +129,11 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, > unsigned int redzone_size; > int redzone_adjust; > > + if (!kasan_stack_collection_enabled()) { > + *flags |= SLAB_KASAN; > + return; > + } > + > /* Add alloc meta. */ > cache->kasan_info.alloc_meta_offset = *size; > *size += sizeof(struct kasan_alloc_meta); > @@ -165,6 +170,8 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, > > size_t kasan_metadata_size(struct kmem_cache *cache) > { > + if (!kasan_stack_collection_enabled()) > + return 0; > return (cache->kasan_info.alloc_meta_offset ? > sizeof(struct kasan_alloc_meta) : 0) + > (cache->kasan_info.free_meta_offset ? > @@ -267,11 +274,13 @@ void * __must_check kasan_init_slab_obj(struct kmem_cache *cache, > { > struct kasan_alloc_meta *alloc_meta; > > - if (!(cache->flags & SLAB_KASAN)) > - return (void *)object; > + if (kasan_stack_collection_enabled()) { > + if (!(cache->flags & SLAB_KASAN)) > + return (void *)object; > > - alloc_meta = kasan_get_alloc_meta(cache, object); > - __memset(alloc_meta, 0, sizeof(*alloc_meta)); > + alloc_meta = kasan_get_alloc_meta(cache, object); > + __memset(alloc_meta, 0, sizeof(*alloc_meta)); > + } > > if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) || IS_ENABLED(CONFIG_KASAN_HW_TAGS)) > object = set_tag(object, assign_tag(cache, object, true, false)); > @@ -308,6 +317,9 @@ static bool __kasan_slab_free(struct kmem_cache *cache, void *object, > rounded_up_size = round_up(cache->object_size, KASAN_GRANULE_SIZE); > kasan_poison_memory(object, rounded_up_size, KASAN_KMALLOC_FREE); > > + if (!kasan_stack_collection_enabled()) > + return false; > + > if ((IS_ENABLED(CONFIG_KASAN_GENERIC) && !quarantine) || > unlikely(!(cache->flags & SLAB_KASAN))) > return false; > @@ -355,7 +367,7 @@ static void *__kasan_kmalloc(struct kmem_cache *cache, const void *object, > kasan_poison_memory((void *)redzone_start, redzone_end - redzone_start, > KASAN_KMALLOC_REDZONE); > > - if (cache->flags & SLAB_KASAN) > + if (kasan_stack_collection_enabled() && (cache->flags & SLAB_KASAN)) > set_alloc_info(cache, (void *)object, flags); > > return set_tag(object, tag); > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c > index 838b29e44e32..2f6f0261af8c 100644 > --- a/mm/kasan/hw_tags.c > +++ b/mm/kasan/hw_tags.c > @@ -8,6 +8,8 @@ > > #define pr_fmt(fmt) "kasan: " fmt > > +#include <linux/init.h> > +#include <linux/jump_label.h> This should include <linux/static_key.h> -- although the rest of the kernel seems to also inconsistently use on or the other. Since the name, as referred to also by macros are "static keys", perhaps the static_key.h header is more appropriate... > #include <linux/kasan.h> > #include <linux/kernel.h> > #include <linux/memory.h> > @@ -17,9 +19,104 @@ > > #include "kasan.h" > > +enum kasan_arg_mode { > + KASAN_ARG_MODE_DEFAULT, > + KASAN_ARG_MODE_OFF, > + KASAN_ARG_MODE_PROD, > + KASAN_ARG_MODE_FULL, > +}; > + > +enum kasan_arg_stacktrace { > + KASAN_ARG_STACKTRACE_DEFAULT, It seems KASAN_ARG_STACKTRACE_DEFAULT is never used explicitly. Could the switch statements just be changed to not have a 'default' but instead refer to *DEFAULT where appropriate? > + KASAN_ARG_STACKTRACE_OFF, > + KASAN_ARG_STACKTRACE_ON, > +}; > + > +enum kasan_arg_fault { > + KASAN_ARG_FAULT_DEFAULT, > + KASAN_ARG_FAULT_REPORT, > + KASAN_ARG_FAULT_PANIC, > +}; > + > +static enum kasan_arg_mode kasan_arg_mode __ro_after_init; > +static enum kasan_arg_stacktrace kasan_arg_stacktrace __ro_after_init; > +static enum kasan_arg_fault kasan_arg_fault __ro_after_init; > + > +/* Whether KASAN is enabled at all. */ > +DEFINE_STATIC_KEY_FALSE_RO(kasan_flag_enabled); > +EXPORT_SYMBOL(kasan_flag_enabled); > + > +/* Whether to collect alloc/free stack traces. */ > +DEFINE_STATIC_KEY_FALSE_RO(kasan_flag_stacktrace); > + > +/* Whether panic or disable tag checking on fault. */ > +bool kasan_flag_panic __ro_after_init; > + > +/* kasan.mode=off/prod/full */ > +static int __init early_kasan_mode(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + if (!strcmp(arg, "off")) > + kasan_arg_mode = KASAN_ARG_MODE_OFF; > + else if (!strcmp(arg, "prod")) > + kasan_arg_mode = KASAN_ARG_MODE_PROD; > + else if (!strcmp(arg, "full")) > + kasan_arg_mode = KASAN_ARG_MODE_FULL; > + else > + return -EINVAL; > + > + return 0; > +} > +early_param("kasan.mode", early_kasan_mode); > + > +/* kasan.stack=off/on */ > +static int __init early_kasan_flag_stacktrace(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + if (!strcmp(arg, "off")) > + kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_OFF; > + else if (!strcmp(arg, "on")) > + kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_ON; > + else > + return -EINVAL; > + > + return 0; > +} > +early_param("kasan.stacktrace", early_kasan_flag_stacktrace); > + > +/* kasan.fault=report/panic */ > +static int __init early_kasan_fault(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + if (!strcmp(arg, "report")) > + kasan_arg_fault = KASAN_ARG_FAULT_REPORT; > + else if (!strcmp(arg, "panic")) > + kasan_arg_fault = KASAN_ARG_FAULT_PANIC; > + else > + return -EINVAL; > + > + return 0; > +} > +early_param("kasan.fault", early_kasan_fault); > + > /* kasan_init_hw_tags_cpu() is called for each CPU. */ > void kasan_init_hw_tags_cpu(void) > { > + /* > + * There's no need to check that the hardware is MTE-capable here, > + * as this function is only called for MTE-capable hardware. > + */ > + > + /* If KASAN is disabled, do nothing. */ > + if (kasan_arg_mode == KASAN_ARG_MODE_OFF) > + return; > + > hw_init_tags(KASAN_TAG_MAX); > hw_enable_tagging(); > } > @@ -27,6 +124,61 @@ void kasan_init_hw_tags_cpu(void) > /* kasan_init_hw_tags() is called once on boot CPU. */ > void kasan_init_hw_tags(void) Is this an __init function, since it sets __ro_after_init vars? > { > + /* If hardware doesn't support MTE, do nothing. */ > + if (!system_supports_mte()) > + return; > + > + /* If KASAN is disabled, do nothing. */ > + if (kasan_arg_mode == KASAN_ARG_MODE_OFF) > + return; This is checked twice, once here and the in the switch. I think remove the one here ^^^. > + /* Choose KASAN mode if kasan boot parameter is not provided. */ > + if (kasan_arg_mode == KASAN_ARG_MODE_DEFAULT) { > + if (IS_ENABLED(CONFIG_DEBUG_KERNEL)) > + kasan_arg_mode = KASAN_ARG_MODE_FULL; > + else > + kasan_arg_mode = KASAN_ARG_MODE_PROD; > + } > + > + /* Preset parameter values based on the mode. */ > + switch (kasan_arg_mode) { > + case KASAN_ARG_MODE_OFF: > + return; > + case KASAN_ARG_MODE_PROD: > + static_branch_enable(&kasan_flag_enabled); > + break; > + case KASAN_ARG_MODE_FULL: > + static_branch_enable(&kasan_flag_enabled); > + static_branch_enable(&kasan_flag_stacktrace); > + break; > + default: I'd suggest removing the 'default' cases in all the switches, so that we get warnings in case we add new options and they aren't handled. Here, having KASAN_ARG_MODE_DEFAULT is probably redundant, but so is 'default' ;-) > + break; > + } > + > + /* Now, optionally override the presets. */ > + > + switch (kasan_arg_stacktrace) { > + case KASAN_ARG_STACKTRACE_OFF: > + static_branch_disable(&kasan_flag_stacktrace); > + break; > + case KASAN_ARG_STACKTRACE_ON: > + static_branch_enable(&kasan_flag_stacktrace); > + break; > + default: > + break; > + } > + > + switch (kasan_arg_fault) { > + case KASAN_ARG_FAULT_REPORT: > + kasan_flag_panic = false; > + break; > + case KASAN_ARG_FAULT_PANIC: > + kasan_flag_panic = true; > + break; > + default: > + break; > + } > + Would be good to get rid of the 'default' cases here. > pr_info("KernelAddressSanitizer initialized\n"); > } > > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > index 2d3c99125996..5eff3d9f624e 100644 > --- a/mm/kasan/kasan.h > +++ b/mm/kasan/kasan.h > @@ -5,6 +5,22 @@ > #include <linux/kasan.h> > #include <linux/stackdepot.h> > > +#ifdef CONFIG_KASAN_HW_TAGS > +#include <linux/jump_label.h> > +DECLARE_STATIC_KEY_FALSE(kasan_flag_stacktrace); > +static inline bool kasan_stack_collection_enabled(void) > +{ > + return static_branch_unlikely(&kasan_flag_stacktrace); > +} > +#else > +static inline bool kasan_stack_collection_enabled(void) > +{ > + return true; > +} > +#endif > + > +extern bool kasan_flag_panic __ro_after_init; > + > #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) > #define KASAN_GRANULE_SIZE (1UL << KASAN_SHADOW_SCALE_SHIFT) > #else > diff --git a/mm/kasan/report.c b/mm/kasan/report.c > index 25ca66c99e48..7d86af340148 100644 > --- a/mm/kasan/report.c > +++ b/mm/kasan/report.c > @@ -99,6 +99,10 @@ static void end_report(unsigned long *flags) > panic_on_warn = 0; > panic("panic_on_warn set ...\n"); > } > +#ifdef CONFIG_KASAN_HW_TAGS > + if (kasan_flag_panic) > + panic("kasan.fault=panic set ...\n"); > +#endif > kasan_enable_current(); > } > > @@ -161,8 +165,8 @@ static void describe_object_addr(struct kmem_cache *cache, void *object, > (void *)(object_addr + cache->object_size)); > } > > -static void describe_object(struct kmem_cache *cache, void *object, > - const void *addr, u8 tag) > +static void describe_object_stacks(struct kmem_cache *cache, void *object, > + const void *addr, u8 tag) > { > struct kasan_alloc_meta *alloc_meta = kasan_get_alloc_meta(cache, object); > > @@ -190,7 +194,13 @@ static void describe_object(struct kmem_cache *cache, void *object, > } > #endif > } > +} > > +static void describe_object(struct kmem_cache *cache, void *object, > + const void *addr, u8 tag) > +{ > + if (kasan_stack_collection_enabled()) > + describe_object_stacks(cache, object, addr, tag); > describe_object_addr(cache, object, addr); > }
On Tue, Nov 10, 2020 at 11:20:15PM +0100, Andrey Konovalov wrote: > Hardware tag-based KASAN mode is intended to eventually be used in > production as a security mitigation. Therefore there's a need for finer > control over KASAN features and for an existence of a kill switch. > > This change adds a few boot parameters for hardware tag-based KASAN that > allow to disable or otherwise control particular KASAN features. > > The features that can be controlled are: > > 1. Whether KASAN is enabled at all. > 2. Whether KASAN collects and saves alloc/free stacks. > 3. Whether KASAN panics on a detected bug or not. > > With this change a new boot parameter kasan.mode allows to choose one of > three main modes: > > - kasan.mode=off - KASAN is disabled, no tag checks are performed > - kasan.mode=prod - only essential production features are enabled > - kasan.mode=full - all KASAN features are enabled Alternative naming if we want to avoid "production" (in case someone considers MTE to be expensive in a production system): - kasan.mode=off - kasan.mode=on - kasan.mode=debug Anyway, whatever you prefer is fine by me: Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On Thu, 12 Nov 2020 at 12:35, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Tue, Nov 10, 2020 at 11:20:15PM +0100, Andrey Konovalov wrote: > > Hardware tag-based KASAN mode is intended to eventually be used in > > production as a security mitigation. Therefore there's a need for finer > > control over KASAN features and for an existence of a kill switch. > > > > This change adds a few boot parameters for hardware tag-based KASAN that > > allow to disable or otherwise control particular KASAN features. > > > > The features that can be controlled are: > > > > 1. Whether KASAN is enabled at all. > > 2. Whether KASAN collects and saves alloc/free stacks. > > 3. Whether KASAN panics on a detected bug or not. > > > > With this change a new boot parameter kasan.mode allows to choose one of > > three main modes: > > > > - kasan.mode=off - KASAN is disabled, no tag checks are performed > > - kasan.mode=prod - only essential production features are enabled > > - kasan.mode=full - all KASAN features are enabled > > Alternative naming if we want to avoid "production" (in case someone > considers MTE to be expensive in a production system): > > - kasan.mode=off > - kasan.mode=on > - kasan.mode=debug I believe this was what it was in RFC, and we had a long discussion on what might be the most intuitive options. Since KASAN is still a debugging tool for the most part, an "on" mode might imply we get all the debugging facilities of regular KASAN. However, this is not the case and misleading. Hence, we decided to be more explicit and avoid "on". > Anyway, whatever you prefer is fine by me: > > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On Thu, Nov 12, 2020 at 12:53:58PM +0100, Marco Elver wrote: > On Thu, 12 Nov 2020 at 12:35, Catalin Marinas <catalin.marinas@arm.com> wrote: > > > > On Tue, Nov 10, 2020 at 11:20:15PM +0100, Andrey Konovalov wrote: > > > Hardware tag-based KASAN mode is intended to eventually be used in > > > production as a security mitigation. Therefore there's a need for finer > > > control over KASAN features and for an existence of a kill switch. > > > > > > This change adds a few boot parameters for hardware tag-based KASAN that > > > allow to disable or otherwise control particular KASAN features. > > > > > > The features that can be controlled are: > > > > > > 1. Whether KASAN is enabled at all. > > > 2. Whether KASAN collects and saves alloc/free stacks. > > > 3. Whether KASAN panics on a detected bug or not. > > > > > > With this change a new boot parameter kasan.mode allows to choose one of > > > three main modes: > > > > > > - kasan.mode=off - KASAN is disabled, no tag checks are performed > > > - kasan.mode=prod - only essential production features are enabled > > > - kasan.mode=full - all KASAN features are enabled > > > > Alternative naming if we want to avoid "production" (in case someone > > considers MTE to be expensive in a production system): > > > > - kasan.mode=off > > - kasan.mode=on > > - kasan.mode=debug > > I believe this was what it was in RFC, and we had a long discussion on > what might be the most intuitive options. Since KASAN is still a > debugging tool for the most part, an "on" mode might imply we get all > the debugging facilities of regular KASAN. However, this is not the > case and misleading. Hence, we decided to be more explicit and avoid > "on". Even better, kasan.mode=fast ;).
On Wed, Nov 11, 2020 at 7:29 PM Marco Elver <elver@google.com> wrote: > > > +#include <linux/init.h> > > +#include <linux/jump_label.h> > > This should include <linux/static_key.h> -- although the rest of the > kernel seems to also inconsistently use on or the other. Since the name, > as referred to also by macros are "static keys", perhaps the > static_key.h header is more appropriate... Will fix. > > +enum kasan_arg_stacktrace { > > + KASAN_ARG_STACKTRACE_DEFAULT, > > It seems KASAN_ARG_STACKTRACE_DEFAULT is never used explicitly. Could > the switch statements just be changed to not have a 'default' but > instead refer to *DEFAULT where appropriate? We need to either cover all cases explicitly, or use default in each switch, otherwise there's a warning. I guess covering everything explicitly is a better approach, in case more values are added in the future, as we'll get warnings for those if they aren't covered in switches. Will do.
On Thu, Nov 12, 2020 at 1:55 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Thu, Nov 12, 2020 at 12:53:58PM +0100, Marco Elver wrote: > > On Thu, 12 Nov 2020 at 12:35, Catalin Marinas <catalin.marinas@arm.com> wrote: > > > > > > On Tue, Nov 10, 2020 at 11:20:15PM +0100, Andrey Konovalov wrote: > > > > Hardware tag-based KASAN mode is intended to eventually be used in > > > > production as a security mitigation. Therefore there's a need for finer > > > > control over KASAN features and for an existence of a kill switch. > > > > > > > > This change adds a few boot parameters for hardware tag-based KASAN that > > > > allow to disable or otherwise control particular KASAN features. > > > > > > > > The features that can be controlled are: > > > > > > > > 1. Whether KASAN is enabled at all. > > > > 2. Whether KASAN collects and saves alloc/free stacks. > > > > 3. Whether KASAN panics on a detected bug or not. > > > > > > > > With this change a new boot parameter kasan.mode allows to choose one of > > > > three main modes: > > > > > > > > - kasan.mode=off - KASAN is disabled, no tag checks are performed > > > > - kasan.mode=prod - only essential production features are enabled > > > > - kasan.mode=full - all KASAN features are enabled > > > > > > Alternative naming if we want to avoid "production" (in case someone > > > considers MTE to be expensive in a production system): > > > > > > - kasan.mode=off > > > - kasan.mode=on > > > - kasan.mode=debug > > > > I believe this was what it was in RFC, and we had a long discussion on > > what might be the most intuitive options. Since KASAN is still a > > debugging tool for the most part, an "on" mode might imply we get all > > the debugging facilities of regular KASAN. However, this is not the > > case and misleading. Hence, we decided to be more explicit and avoid > > "on". > > Even better, kasan.mode=fast ;). Well, it uses sync, so technically it's not as fast as it could be with async :)
On Tue, Nov 10, 2020 at 11:20PM +0100, 'Andrey Konovalov' via kasan-dev wrote: [...] > +/* kasan.mode=off/prod/full */ > +static int __init early_kasan_mode(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + if (!strcmp(arg, "off")) > + kasan_arg_mode = KASAN_ARG_MODE_OFF; > + else if (!strcmp(arg, "prod")) > + kasan_arg_mode = KASAN_ARG_MODE_PROD; > + else if (!strcmp(arg, "full")) > + kasan_arg_mode = KASAN_ARG_MODE_FULL; > + else > + return -EINVAL; > + > + return 0; > +} > +early_param("kasan.mode", early_kasan_mode); > + > +/* kasan.stack=off/on */ > +static int __init early_kasan_flag_stacktrace(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + if (!strcmp(arg, "off")) > + kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_OFF; > + else if (!strcmp(arg, "on")) > + kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_ON; > + else > + return -EINVAL; > + > + return 0; > +} > +early_param("kasan.stacktrace", early_kasan_flag_stacktrace); > + > +/* kasan.fault=report/panic */ > +static int __init early_kasan_fault(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + if (!strcmp(arg, "report")) > + kasan_arg_fault = KASAN_ARG_FAULT_REPORT; > + else if (!strcmp(arg, "panic")) > + kasan_arg_fault = KASAN_ARG_FAULT_PANIC; > + else > + return -EINVAL; > + > + return 0; > +} [...] The above could be simplified, see suggestion below. Thanks, -- Marco ------ >8 ------ diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c index c91f2c06ecb5..71fc481ad21d 100644 --- a/mm/kasan/hw_tags.c +++ b/mm/kasan/hw_tags.c @@ -64,10 +64,8 @@ static int __init early_kasan_mode(char *arg) kasan_arg_mode = KASAN_ARG_MODE_PROD; else if (!strcmp(arg, "full")) kasan_arg_mode = KASAN_ARG_MODE_FULL; - else - return -EINVAL; - return 0; + return -EINVAL; } early_param("kasan.mode", early_kasan_mode); @@ -81,10 +79,8 @@ static int __init early_kasan_flag_stacktrace(char *arg) kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_OFF; else if (!strcmp(arg, "on")) kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_ON; - else - return -EINVAL; - return 0; + return -EINVAL; } early_param("kasan.stacktrace", early_kasan_flag_stacktrace); @@ -98,10 +94,8 @@ static int __init early_kasan_fault(char *arg) kasan_arg_fault = KASAN_ARG_FAULT_REPORT; else if (!strcmp(arg, "panic")) kasan_arg_fault = KASAN_ARG_FAULT_PANIC; - else - return -EINVAL; - return 0; + return -EINVAL; } early_param("kasan.fault", early_kasan_fault);
On Fri, Nov 13, 2020 at 06:52PM +0100, Marco Elver wrote: > On Tue, Nov 10, 2020 at 11:20PM +0100, 'Andrey Konovalov' via kasan-dev wrote: > [...] > > +/* kasan.mode=off/prod/full */ > > +static int __init early_kasan_mode(char *arg) > > +{ > > + if (!arg) > > + return -EINVAL; > > + > > + if (!strcmp(arg, "off")) > > + kasan_arg_mode = KASAN_ARG_MODE_OFF; > > + else if (!strcmp(arg, "prod")) > > + kasan_arg_mode = KASAN_ARG_MODE_PROD; > > + else if (!strcmp(arg, "full")) > > + kasan_arg_mode = KASAN_ARG_MODE_FULL; > > + else > > + return -EINVAL; > > + > > + return 0; > > +} > > +early_param("kasan.mode", early_kasan_mode); > > + > > +/* kasan.stack=off/on */ > > +static int __init early_kasan_flag_stacktrace(char *arg) > > +{ > > + if (!arg) > > + return -EINVAL; > > + > > + if (!strcmp(arg, "off")) > > + kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_OFF; > > + else if (!strcmp(arg, "on")) > > + kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_ON; > > + else > > + return -EINVAL; > > + > > + return 0; > > +} > > +early_param("kasan.stacktrace", early_kasan_flag_stacktrace); > > + > > +/* kasan.fault=report/panic */ > > +static int __init early_kasan_fault(char *arg) > > +{ > > + if (!arg) > > + return -EINVAL; > > + > > + if (!strcmp(arg, "report")) > > + kasan_arg_fault = KASAN_ARG_FAULT_REPORT; > > + else if (!strcmp(arg, "panic")) > > + kasan_arg_fault = KASAN_ARG_FAULT_PANIC; > > + else > > + return -EINVAL; > > + > > + return 0; > > +} > [...] > > The above could be simplified, see suggestion below. > > Thanks, > -- Marco > > ------ >8 ------ > > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c > index c91f2c06ecb5..71fc481ad21d 100644 > --- a/mm/kasan/hw_tags.c > +++ b/mm/kasan/hw_tags.c > @@ -64,10 +64,8 @@ static int __init early_kasan_mode(char *arg) > kasan_arg_mode = KASAN_ARG_MODE_PROD; > else if (!strcmp(arg, "full")) > kasan_arg_mode = KASAN_ARG_MODE_FULL; > - else > - return -EINVAL; > > - return 0; > + return -EINVAL; Ah that clearly doesn't work. Hmm, never mind this suggestion, sorry. Thanks, -- Marco
diff --git a/mm/kasan/common.c b/mm/kasan/common.c index 4598c1364f19..efad5ed6a3bd 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -129,6 +129,11 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, unsigned int redzone_size; int redzone_adjust; + if (!kasan_stack_collection_enabled()) { + *flags |= SLAB_KASAN; + return; + } + /* Add alloc meta. */ cache->kasan_info.alloc_meta_offset = *size; *size += sizeof(struct kasan_alloc_meta); @@ -165,6 +170,8 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, size_t kasan_metadata_size(struct kmem_cache *cache) { + if (!kasan_stack_collection_enabled()) + return 0; return (cache->kasan_info.alloc_meta_offset ? sizeof(struct kasan_alloc_meta) : 0) + (cache->kasan_info.free_meta_offset ? @@ -267,11 +274,13 @@ void * __must_check kasan_init_slab_obj(struct kmem_cache *cache, { struct kasan_alloc_meta *alloc_meta; - if (!(cache->flags & SLAB_KASAN)) - return (void *)object; + if (kasan_stack_collection_enabled()) { + if (!(cache->flags & SLAB_KASAN)) + return (void *)object; - alloc_meta = kasan_get_alloc_meta(cache, object); - __memset(alloc_meta, 0, sizeof(*alloc_meta)); + alloc_meta = kasan_get_alloc_meta(cache, object); + __memset(alloc_meta, 0, sizeof(*alloc_meta)); + } if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) || IS_ENABLED(CONFIG_KASAN_HW_TAGS)) object = set_tag(object, assign_tag(cache, object, true, false)); @@ -308,6 +317,9 @@ static bool __kasan_slab_free(struct kmem_cache *cache, void *object, rounded_up_size = round_up(cache->object_size, KASAN_GRANULE_SIZE); kasan_poison_memory(object, rounded_up_size, KASAN_KMALLOC_FREE); + if (!kasan_stack_collection_enabled()) + return false; + if ((IS_ENABLED(CONFIG_KASAN_GENERIC) && !quarantine) || unlikely(!(cache->flags & SLAB_KASAN))) return false; @@ -355,7 +367,7 @@ static void *__kasan_kmalloc(struct kmem_cache *cache, const void *object, kasan_poison_memory((void *)redzone_start, redzone_end - redzone_start, KASAN_KMALLOC_REDZONE); - if (cache->flags & SLAB_KASAN) + if (kasan_stack_collection_enabled() && (cache->flags & SLAB_KASAN)) set_alloc_info(cache, (void *)object, flags); return set_tag(object, tag); diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c index 838b29e44e32..2f6f0261af8c 100644 --- a/mm/kasan/hw_tags.c +++ b/mm/kasan/hw_tags.c @@ -8,6 +8,8 @@ #define pr_fmt(fmt) "kasan: " fmt +#include <linux/init.h> +#include <linux/jump_label.h> #include <linux/kasan.h> #include <linux/kernel.h> #include <linux/memory.h> @@ -17,9 +19,104 @@ #include "kasan.h" +enum kasan_arg_mode { + KASAN_ARG_MODE_DEFAULT, + KASAN_ARG_MODE_OFF, + KASAN_ARG_MODE_PROD, + KASAN_ARG_MODE_FULL, +}; + +enum kasan_arg_stacktrace { + KASAN_ARG_STACKTRACE_DEFAULT, + KASAN_ARG_STACKTRACE_OFF, + KASAN_ARG_STACKTRACE_ON, +}; + +enum kasan_arg_fault { + KASAN_ARG_FAULT_DEFAULT, + KASAN_ARG_FAULT_REPORT, + KASAN_ARG_FAULT_PANIC, +}; + +static enum kasan_arg_mode kasan_arg_mode __ro_after_init; +static enum kasan_arg_stacktrace kasan_arg_stacktrace __ro_after_init; +static enum kasan_arg_fault kasan_arg_fault __ro_after_init; + +/* Whether KASAN is enabled at all. */ +DEFINE_STATIC_KEY_FALSE_RO(kasan_flag_enabled); +EXPORT_SYMBOL(kasan_flag_enabled); + +/* Whether to collect alloc/free stack traces. */ +DEFINE_STATIC_KEY_FALSE_RO(kasan_flag_stacktrace); + +/* Whether panic or disable tag checking on fault. */ +bool kasan_flag_panic __ro_after_init; + +/* kasan.mode=off/prod/full */ +static int __init early_kasan_mode(char *arg) +{ + if (!arg) + return -EINVAL; + + if (!strcmp(arg, "off")) + kasan_arg_mode = KASAN_ARG_MODE_OFF; + else if (!strcmp(arg, "prod")) + kasan_arg_mode = KASAN_ARG_MODE_PROD; + else if (!strcmp(arg, "full")) + kasan_arg_mode = KASAN_ARG_MODE_FULL; + else + return -EINVAL; + + return 0; +} +early_param("kasan.mode", early_kasan_mode); + +/* kasan.stack=off/on */ +static int __init early_kasan_flag_stacktrace(char *arg) +{ + if (!arg) + return -EINVAL; + + if (!strcmp(arg, "off")) + kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_OFF; + else if (!strcmp(arg, "on")) + kasan_arg_stacktrace = KASAN_ARG_STACKTRACE_ON; + else + return -EINVAL; + + return 0; +} +early_param("kasan.stacktrace", early_kasan_flag_stacktrace); + +/* kasan.fault=report/panic */ +static int __init early_kasan_fault(char *arg) +{ + if (!arg) + return -EINVAL; + + if (!strcmp(arg, "report")) + kasan_arg_fault = KASAN_ARG_FAULT_REPORT; + else if (!strcmp(arg, "panic")) + kasan_arg_fault = KASAN_ARG_FAULT_PANIC; + else + return -EINVAL; + + return 0; +} +early_param("kasan.fault", early_kasan_fault); + /* kasan_init_hw_tags_cpu() is called for each CPU. */ void kasan_init_hw_tags_cpu(void) { + /* + * There's no need to check that the hardware is MTE-capable here, + * as this function is only called for MTE-capable hardware. + */ + + /* If KASAN is disabled, do nothing. */ + if (kasan_arg_mode == KASAN_ARG_MODE_OFF) + return; + hw_init_tags(KASAN_TAG_MAX); hw_enable_tagging(); } @@ -27,6 +124,61 @@ void kasan_init_hw_tags_cpu(void) /* kasan_init_hw_tags() is called once on boot CPU. */ void kasan_init_hw_tags(void) { + /* If hardware doesn't support MTE, do nothing. */ + if (!system_supports_mte()) + return; + + /* If KASAN is disabled, do nothing. */ + if (kasan_arg_mode == KASAN_ARG_MODE_OFF) + return; + + /* Choose KASAN mode if kasan boot parameter is not provided. */ + if (kasan_arg_mode == KASAN_ARG_MODE_DEFAULT) { + if (IS_ENABLED(CONFIG_DEBUG_KERNEL)) + kasan_arg_mode = KASAN_ARG_MODE_FULL; + else + kasan_arg_mode = KASAN_ARG_MODE_PROD; + } + + /* Preset parameter values based on the mode. */ + switch (kasan_arg_mode) { + case KASAN_ARG_MODE_OFF: + return; + case KASAN_ARG_MODE_PROD: + static_branch_enable(&kasan_flag_enabled); + break; + case KASAN_ARG_MODE_FULL: + static_branch_enable(&kasan_flag_enabled); + static_branch_enable(&kasan_flag_stacktrace); + break; + default: + break; + } + + /* Now, optionally override the presets. */ + + switch (kasan_arg_stacktrace) { + case KASAN_ARG_STACKTRACE_OFF: + static_branch_disable(&kasan_flag_stacktrace); + break; + case KASAN_ARG_STACKTRACE_ON: + static_branch_enable(&kasan_flag_stacktrace); + break; + default: + break; + } + + switch (kasan_arg_fault) { + case KASAN_ARG_FAULT_REPORT: + kasan_flag_panic = false; + break; + case KASAN_ARG_FAULT_PANIC: + kasan_flag_panic = true; + break; + default: + break; + } + pr_info("KernelAddressSanitizer initialized\n"); } diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h index 2d3c99125996..5eff3d9f624e 100644 --- a/mm/kasan/kasan.h +++ b/mm/kasan/kasan.h @@ -5,6 +5,22 @@ #include <linux/kasan.h> #include <linux/stackdepot.h> +#ifdef CONFIG_KASAN_HW_TAGS +#include <linux/jump_label.h> +DECLARE_STATIC_KEY_FALSE(kasan_flag_stacktrace); +static inline bool kasan_stack_collection_enabled(void) +{ + return static_branch_unlikely(&kasan_flag_stacktrace); +} +#else +static inline bool kasan_stack_collection_enabled(void) +{ + return true; +} +#endif + +extern bool kasan_flag_panic __ro_after_init; + #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) #define KASAN_GRANULE_SIZE (1UL << KASAN_SHADOW_SCALE_SHIFT) #else diff --git a/mm/kasan/report.c b/mm/kasan/report.c index 25ca66c99e48..7d86af340148 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -99,6 +99,10 @@ static void end_report(unsigned long *flags) panic_on_warn = 0; panic("panic_on_warn set ...\n"); } +#ifdef CONFIG_KASAN_HW_TAGS + if (kasan_flag_panic) + panic("kasan.fault=panic set ...\n"); +#endif kasan_enable_current(); } @@ -161,8 +165,8 @@ static void describe_object_addr(struct kmem_cache *cache, void *object, (void *)(object_addr + cache->object_size)); } -static void describe_object(struct kmem_cache *cache, void *object, - const void *addr, u8 tag) +static void describe_object_stacks(struct kmem_cache *cache, void *object, + const void *addr, u8 tag) { struct kasan_alloc_meta *alloc_meta = kasan_get_alloc_meta(cache, object); @@ -190,7 +194,13 @@ static void describe_object(struct kmem_cache *cache, void *object, } #endif } +} +static void describe_object(struct kmem_cache *cache, void *object, + const void *addr, u8 tag) +{ + if (kasan_stack_collection_enabled()) + describe_object_stacks(cache, object, addr, tag); describe_object_addr(cache, object, addr); }
Hardware tag-based KASAN mode is intended to eventually be used in production as a security mitigation. Therefore there's a need for finer control over KASAN features and for an existence of a kill switch. This change adds a few boot parameters for hardware tag-based KASAN that allow to disable or otherwise control particular KASAN features. The features that can be controlled are: 1. Whether KASAN is enabled at all. 2. Whether KASAN collects and saves alloc/free stacks. 3. Whether KASAN panics on a detected bug or not. With this change a new boot parameter kasan.mode allows to choose one of three main modes: - kasan.mode=off - KASAN is disabled, no tag checks are performed - kasan.mode=prod - only essential production features are enabled - kasan.mode=full - all KASAN features are enabled The chosen mode provides default control values for the features mentioned above. However it's also possible to override the default values by providing: - kasan.stacktrace=off/on - enable alloc/free stack collection (default: on for mode=full, otherwise off) - kasan.fault=report/panic - only report tag fault or also panic (default: report) If kasan.mode parameter is not provided, it defaults to full when CONFIG_DEBUG_KERNEL is enabled, and to prod otherwise. It is essential that switching between these modes doesn't require rebuilding the kernel with different configs, as this is required by the Android GKI (Generic Kernel Image) initiative [1]. [1] https://source.android.com/devices/architecture/kernel/generic-kernel-image Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Link: https://linux-review.googlesource.com/id/If7d37003875b2ed3e0935702c8015c223d6416a4 --- mm/kasan/common.c | 22 +++++-- mm/kasan/hw_tags.c | 152 +++++++++++++++++++++++++++++++++++++++++++++ mm/kasan/kasan.h | 16 +++++ mm/kasan/report.c | 14 ++++- 4 files changed, 197 insertions(+), 7 deletions(-)