| Message ID | 7e95d4739f5617b2c1acf52f37e01f1ca83750b5.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 wrote: > There's a config option CONFIG_KASAN_STACK that has to be enabled for > KASAN to use stack instrumentation and perform validity checks for > stack variables. > > There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. > Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is > enabled. > > Note, that CONFIG_KASAN_STACK is an option that is currently always > defined when CONFIG_KASAN is enabled, and therefore has to be tested > with #if instead of #ifdef. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > Link: https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3 > --- > arch/arm64/kernel/sleep.S | 2 +- > arch/x86/kernel/acpi/wakeup_64.S | 2 +- > include/linux/kasan.h | 10 ++++++---- > mm/kasan/common.c | 2 ++ > 4 files changed, 10 insertions(+), 6 deletions(-) Reviewed-by: Marco Elver <elver@google.com> > diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S > index ba40d57757d6..bdadfa56b40e 100644 > --- a/arch/arm64/kernel/sleep.S > +++ b/arch/arm64/kernel/sleep.S > @@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume) > */ > bl cpu_do_resume > > -#ifdef CONFIG_KASAN > +#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK > mov x0, sp > bl kasan_unpoison_task_stack_below > #endif > diff --git a/arch/x86/kernel/acpi/wakeup_64.S b/arch/x86/kernel/acpi/wakeup_64.S > index c8daa92f38dc..5d3a0b8fd379 100644 > --- a/arch/x86/kernel/acpi/wakeup_64.S > +++ b/arch/x86/kernel/acpi/wakeup_64.S > @@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel) > movq pt_regs_r14(%rax), %r14 > movq pt_regs_r15(%rax), %r15 > > -#ifdef CONFIG_KASAN > +#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK > /* > * The suspend path may have poisoned some areas deeper in the stack, > * which we now need to unpoison. > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > index f22bdef82111..b9b9db335d87 100644 > --- a/include/linux/kasan.h > +++ b/include/linux/kasan.h > @@ -76,8 +76,6 @@ static inline void kasan_disable_current(void) {} > > void kasan_unpoison_memory(const void *address, size_t size); > > -void kasan_unpoison_task_stack(struct task_struct *task); > - > void kasan_alloc_pages(struct page *page, unsigned int order); > void kasan_free_pages(struct page *page, unsigned int order); > > @@ -122,8 +120,6 @@ void kasan_restore_multi_shot(bool enabled); > > static inline void kasan_unpoison_memory(const void *address, size_t size) {} > > -static inline void kasan_unpoison_task_stack(struct task_struct *task) {} > - > static inline void kasan_alloc_pages(struct page *page, unsigned int order) {} > static inline void kasan_free_pages(struct page *page, unsigned int order) {} > > @@ -175,6 +171,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } > > #endif /* CONFIG_KASAN */ > > +#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK > +void kasan_unpoison_task_stack(struct task_struct *task); > +#else > +static inline void kasan_unpoison_task_stack(struct task_struct *task) {} > +#endif > + > #ifdef CONFIG_KASAN_GENERIC > > void kasan_cache_shrink(struct kmem_cache *cache); > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > index a880e5a547ed..a3e67d49b893 100644 > --- a/mm/kasan/common.c > +++ b/mm/kasan/common.c > @@ -58,6 +58,7 @@ void kasan_disable_current(void) > } > #endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */ > > +#if CONFIG_KASAN_STACK > static void __kasan_unpoison_stack(struct task_struct *task, const void *sp) > { > void *base = task_stack_page(task); > @@ -84,6 +85,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark) > > kasan_unpoison_memory(base, watermark - base); > } > +#endif /* CONFIG_KASAN_STACK */ > > void kasan_alloc_pages(struct page *page, unsigned int order) > { > -- > 2.29.2.222.g5d2a92d10f8-goog >
On Tue, Nov 10, 2020 at 11:20:08PM +0100, Andrey Konovalov wrote: > There's a config option CONFIG_KASAN_STACK that has to be enabled for > KASAN to use stack instrumentation and perform validity checks for > stack variables. > > There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. > Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is > enabled. > > Note, that CONFIG_KASAN_STACK is an option that is currently always > defined when CONFIG_KASAN is enabled, and therefore has to be tested > with #if instead of #ifdef. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > Link: https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3 > --- > arch/arm64/kernel/sleep.S | 2 +- > arch/x86/kernel/acpi/wakeup_64.S | 2 +- > include/linux/kasan.h | 10 ++++++---- > mm/kasan/common.c | 2 ++ > 4 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S > index ba40d57757d6..bdadfa56b40e 100644 > --- a/arch/arm64/kernel/sleep.S > +++ b/arch/arm64/kernel/sleep.S > @@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume) > */ > bl cpu_do_resume > > -#ifdef CONFIG_KASAN > +#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK > mov x0, sp > bl kasan_unpoison_task_stack_below > #endif I don't understand why CONFIG_KASAN_STACK is not a bool (do you plan to add more values to it?) but for arm64: Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On Thu, Nov 12, 2020 at 10:51 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Tue, Nov 10, 2020 at 11:20:08PM +0100, Andrey Konovalov wrote: > > There's a config option CONFIG_KASAN_STACK that has to be enabled for > > KASAN to use stack instrumentation and perform validity checks for > > stack variables. > > > > There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. > > Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is > > enabled. > > > > Note, that CONFIG_KASAN_STACK is an option that is currently always > > defined when CONFIG_KASAN is enabled, and therefore has to be tested > > with #if instead of #ifdef. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > Link: https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3 > > --- > > arch/arm64/kernel/sleep.S | 2 +- > > arch/x86/kernel/acpi/wakeup_64.S | 2 +- > > include/linux/kasan.h | 10 ++++++---- > > mm/kasan/common.c | 2 ++ > > 4 files changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S > > index ba40d57757d6..bdadfa56b40e 100644 > > --- a/arch/arm64/kernel/sleep.S > > +++ b/arch/arm64/kernel/sleep.S > > @@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume) > > */ > > bl cpu_do_resume > > > > -#ifdef CONFIG_KASAN > > +#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK > > mov x0, sp > > bl kasan_unpoison_task_stack_below > > #endif > > I don't understand why CONFIG_KASAN_STACK is not a bool (do you plan to > add more values to it?) but for arm64: I don't remember if there's an actual reason. Perhaps this is something that can be reworked later, but I don't want to get into this in this series. > Acked-by: Catalin Marinas <catalin.marinas@arm.com> Thanks!
diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S index ba40d57757d6..bdadfa56b40e 100644 --- a/arch/arm64/kernel/sleep.S +++ b/arch/arm64/kernel/sleep.S @@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume) */ bl cpu_do_resume -#ifdef CONFIG_KASAN +#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK mov x0, sp bl kasan_unpoison_task_stack_below #endif diff --git a/arch/x86/kernel/acpi/wakeup_64.S b/arch/x86/kernel/acpi/wakeup_64.S index c8daa92f38dc..5d3a0b8fd379 100644 --- a/arch/x86/kernel/acpi/wakeup_64.S +++ b/arch/x86/kernel/acpi/wakeup_64.S @@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel) movq pt_regs_r14(%rax), %r14 movq pt_regs_r15(%rax), %r15 -#ifdef CONFIG_KASAN +#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK /* * The suspend path may have poisoned some areas deeper in the stack, * which we now need to unpoison. diff --git a/include/linux/kasan.h b/include/linux/kasan.h index f22bdef82111..b9b9db335d87 100644 --- a/include/linux/kasan.h +++ b/include/linux/kasan.h @@ -76,8 +76,6 @@ static inline void kasan_disable_current(void) {} void kasan_unpoison_memory(const void *address, size_t size); -void kasan_unpoison_task_stack(struct task_struct *task); - void kasan_alloc_pages(struct page *page, unsigned int order); void kasan_free_pages(struct page *page, unsigned int order); @@ -122,8 +120,6 @@ void kasan_restore_multi_shot(bool enabled); static inline void kasan_unpoison_memory(const void *address, size_t size) {} -static inline void kasan_unpoison_task_stack(struct task_struct *task) {} - static inline void kasan_alloc_pages(struct page *page, unsigned int order) {} static inline void kasan_free_pages(struct page *page, unsigned int order) {} @@ -175,6 +171,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } #endif /* CONFIG_KASAN */ +#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK +void kasan_unpoison_task_stack(struct task_struct *task); +#else +static inline void kasan_unpoison_task_stack(struct task_struct *task) {} +#endif + #ifdef CONFIG_KASAN_GENERIC void kasan_cache_shrink(struct kmem_cache *cache); diff --git a/mm/kasan/common.c b/mm/kasan/common.c index a880e5a547ed..a3e67d49b893 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -58,6 +58,7 @@ void kasan_disable_current(void) } #endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */ +#if CONFIG_KASAN_STACK static void __kasan_unpoison_stack(struct task_struct *task, const void *sp) { void *base = task_stack_page(task); @@ -84,6 +85,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark) kasan_unpoison_memory(base, watermark - base); } +#endif /* CONFIG_KASAN_STACK */ void kasan_alloc_pages(struct page *page, unsigned int order) {
There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables. There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled. Note, that CONFIG_KASAN_STACK is an option that is currently always defined when CONFIG_KASAN is enabled, and therefore has to be tested with #if instead of #ifdef. Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Link: https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3 --- arch/arm64/kernel/sleep.S | 2 +- arch/x86/kernel/acpi/wakeup_64.S | 2 +- include/linux/kasan.h | 10 ++++++---- mm/kasan/common.c | 2 ++ 4 files changed, 10 insertions(+), 6 deletions(-)