* [PATCH v2 0/2] scs: switch to vmapped shadow stacks @ 2020-11-24 19:59 Sami Tolvanen 2020-11-24 19:59 ` [PATCH v2 1/2] " Sami Tolvanen 2020-11-24 19:59 ` [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI " Sami Tolvanen 0 siblings, 2 replies; 10+ messages in thread From: Sami Tolvanen @ 2020-11-24 19:59 UTC (permalink / raw) To: Will Deacon, Catalin Marinas Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, linux-kernel, James Morse, Sami Tolvanen, linux-arm-kernel As discussed a few months ago [1][2], virtually mapped shadow call stacks are better for safety and robustness. This series dusts off the VMAP option from the original SCS patch series and switches the kernel to use virtually mapped shadow stacks unconditionally when SCS is enabled. [1] https://lore.kernel.org/lkml/20200515172355.GD23334@willie-the-truck/ [2] https://lore.kernel.org/lkml/20200427220942.GB80713@google.com/ Changes in v2: - Added SCS_ORDER and used it to define SCS_SIZE, switched vmalloc() to use SCS_SIZE and removed the alignment. - Moved the kasan_unpoison_vmalloc() to scs_alloc() when using a cached shadow stack instead of calling it in scs_free(). - Added a comment to scs_free(). - Moved arm64 IRQ and SDEI shadow stack initialization to irq/sdei.c, and removed the now unneeded scs.c. Sami Tolvanen (2): scs: switch to vmapped shadow stacks arm64: scs: use vmapped IRQ and SDEI shadow stacks arch/arm64/kernel/Makefile | 1 - arch/arm64/kernel/entry.S | 6 ++-- arch/arm64/kernel/irq.c | 19 ++++++++++ arch/arm64/kernel/scs.c | 16 --------- arch/arm64/kernel/sdei.c | 71 +++++++++++++++++++++++++++++++------- include/linux/scs.h | 16 ++++----- kernel/scs.c | 66 +++++++++++++++++++++++++++++------ 7 files changed, 142 insertions(+), 53 deletions(-) delete mode 100644 arch/arm64/kernel/scs.c base-commit: d5beb3140f91b1c8a3d41b14d729aefa4dcc58bc -- 2.29.2.454.gaff20da3a2-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] scs: switch to vmapped shadow stacks 2020-11-24 19:59 [PATCH v2 0/2] scs: switch to vmapped shadow stacks Sami Tolvanen @ 2020-11-24 19:59 ` Sami Tolvanen 2020-11-24 22:04 ` Kees Cook 2020-11-30 11:44 ` Will Deacon 2020-11-24 19:59 ` [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI " Sami Tolvanen 1 sibling, 2 replies; 10+ messages in thread From: Sami Tolvanen @ 2020-11-24 19:59 UTC (permalink / raw) To: Will Deacon, Catalin Marinas Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, linux-kernel, James Morse, Sami Tolvanen, linux-arm-kernel The kernel currently uses kmem_cache to allocate shadow call stacks, which means an overflows may not be immediately detected and can potentially result in another task's shadow stack to be overwritten. This change switches SCS to use virtually mapped shadow stacks for tasks, which increases shadow stack size to a full page and provides more robust overflow detection, similarly to VMAP_STACK. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- include/linux/scs.h | 12 ++++----- kernel/scs.c | 66 +++++++++++++++++++++++++++++++++++++-------- 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/include/linux/scs.h b/include/linux/scs.h index 6dec390cf154..2a506c2a16f4 100644 --- a/include/linux/scs.h +++ b/include/linux/scs.h @@ -15,12 +15,8 @@ #ifdef CONFIG_SHADOW_CALL_STACK -/* - * In testing, 1 KiB shadow stack size (i.e. 128 stack frames on a 64-bit - * architecture) provided ~40% safety margin on stack usage while keeping - * memory allocation overhead reasonable. - */ -#define SCS_SIZE SZ_1K +#define SCS_ORDER 0 +#define SCS_SIZE (PAGE_SIZE << SCS_ORDER) #define GFP_SCS (GFP_KERNEL | __GFP_ZERO) /* An illegal pointer value to mark the end of the shadow stack. */ @@ -33,6 +29,8 @@ #define task_scs(tsk) (task_thread_info(tsk)->scs_base) #define task_scs_sp(tsk) (task_thread_info(tsk)->scs_sp) +void *scs_alloc(int node); +void scs_free(void *s); void scs_init(void); int scs_prepare(struct task_struct *tsk, int node); void scs_release(struct task_struct *tsk); @@ -61,6 +59,8 @@ static inline bool task_scs_end_corrupted(struct task_struct *tsk) #else /* CONFIG_SHADOW_CALL_STACK */ +static inline void *scs_alloc(int node) { return NULL; } +static inline void scs_free(void *s) {} static inline void scs_init(void) {} static inline void scs_task_reset(struct task_struct *tsk) {} static inline int scs_prepare(struct task_struct *tsk, int node) { return 0; } diff --git a/kernel/scs.c b/kernel/scs.c index 4ff4a7ba0094..25b0dd5aa0e2 100644 --- a/kernel/scs.c +++ b/kernel/scs.c @@ -5,50 +5,94 @@ * Copyright (C) 2019 Google LLC */ +#include <linux/cpuhotplug.h> #include <linux/kasan.h> #include <linux/mm.h> #include <linux/scs.h> -#include <linux/slab.h> +#include <linux/vmalloc.h> #include <linux/vmstat.h> -static struct kmem_cache *scs_cache; - static void __scs_account(void *s, int account) { - struct page *scs_page = virt_to_page(s); + struct page *scs_page = vmalloc_to_page(s); mod_node_page_state(page_pgdat(scs_page), NR_KERNEL_SCS_KB, account * (SCS_SIZE / SZ_1K)); } -static void *scs_alloc(int node) +/* Matches NR_CACHED_STACKS for VMAP_STACK */ +#define NR_CACHED_SCS 2 +static DEFINE_PER_CPU(void *, scs_cache[NR_CACHED_SCS]); + +void *scs_alloc(int node) { - void *s = kmem_cache_alloc_node(scs_cache, GFP_SCS, node); + int i; + void *s; + + for (i = 0; i < NR_CACHED_SCS; i++) { + s = this_cpu_xchg(scs_cache[i], NULL); + if (s) { + kasan_unpoison_vmalloc(s, SCS_SIZE); + memset(s, 0, SCS_SIZE); + goto out; + } + } + + s = __vmalloc_node_range(SCS_SIZE, 1, VMALLOC_START, VMALLOC_END, + GFP_SCS, PAGE_KERNEL, 0, node, + __builtin_return_address(0)); if (!s) return NULL; +out: *__scs_magic(s) = SCS_END_MAGIC; /* * Poison the allocation to catch unintentional accesses to * the shadow stack when KASAN is enabled. */ - kasan_poison_object_data(scs_cache, s); + kasan_poison_vmalloc(s, SCS_SIZE); __scs_account(s, 1); return s; } -static void scs_free(void *s) +void scs_free(void *s) { + int i; + __scs_account(s, -1); - kasan_unpoison_object_data(scs_cache, s); - kmem_cache_free(scs_cache, s); + + /* + * We cannot sleep as this can be called in interrupt context, + * so use this_cpu_cmpxchg to update the cache, and vfree_atomic + * to free the stack. + */ + + for (i = 0; i < NR_CACHED_SCS; i++) + if (this_cpu_cmpxchg(scs_cache[i], 0, s) == NULL) + return; + + vfree_atomic(s); +} + +static int scs_cleanup(unsigned int cpu) +{ + int i; + void **cache = per_cpu_ptr(scs_cache, cpu); + + for (i = 0; i < NR_CACHED_SCS; i++) { + vfree(cache[i]); + cache[i] = NULL; + } + + return 0; } void __init scs_init(void) { - scs_cache = kmem_cache_create("scs_cache", SCS_SIZE, 0, 0, NULL); + cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "scs:scs_cache", NULL, + scs_cleanup); } int scs_prepare(struct task_struct *tsk, int node) -- 2.29.2.454.gaff20da3a2-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] scs: switch to vmapped shadow stacks 2020-11-24 19:59 ` [PATCH v2 1/2] " Sami Tolvanen @ 2020-11-24 22:04 ` Kees Cook 2020-11-30 11:44 ` Will Deacon 1 sibling, 0 replies; 10+ messages in thread From: Kees Cook @ 2020-11-24 22:04 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Ard Biesheuvel, Catalin Marinas, linux-kernel, James Morse, Will Deacon, linux-arm-kernel On Tue, Nov 24, 2020 at 11:59:39AM -0800, Sami Tolvanen wrote: > The kernel currently uses kmem_cache to allocate shadow call stacks, > which means an overflows may not be immediately detected and can > potentially result in another task's shadow stack to be overwritten. > > This change switches SCS to use virtually mapped shadow stacks for > tasks, which increases shadow stack size to a full page and provides > more robust overflow detection, similarly to VMAP_STACK. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] scs: switch to vmapped shadow stacks 2020-11-24 19:59 ` [PATCH v2 1/2] " Sami Tolvanen 2020-11-24 22:04 ` Kees Cook @ 2020-11-30 11:44 ` Will Deacon 2020-11-30 20:03 ` Sami Tolvanen 1 sibling, 1 reply; 10+ messages in thread From: Will Deacon @ 2020-11-30 11:44 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, Catalin Marinas, linux-kernel, James Morse, linux-arm-kernel On Tue, Nov 24, 2020 at 11:59:39AM -0800, Sami Tolvanen wrote: > The kernel currently uses kmem_cache to allocate shadow call stacks, > which means an overflows may not be immediately detected and can > potentially result in another task's shadow stack to be overwritten. > > This change switches SCS to use virtually mapped shadow stacks for > tasks, which increases shadow stack size to a full page and provides > more robust overflow detection, similarly to VMAP_STACK. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > include/linux/scs.h | 12 ++++----- > kernel/scs.c | 66 +++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 61 insertions(+), 17 deletions(-) > > diff --git a/include/linux/scs.h b/include/linux/scs.h > index 6dec390cf154..2a506c2a16f4 100644 > --- a/include/linux/scs.h > +++ b/include/linux/scs.h > @@ -15,12 +15,8 @@ > > #ifdef CONFIG_SHADOW_CALL_STACK > > -/* > - * In testing, 1 KiB shadow stack size (i.e. 128 stack frames on a 64-bit > - * architecture) provided ~40% safety margin on stack usage while keeping > - * memory allocation overhead reasonable. > - */ > -#define SCS_SIZE SZ_1K > +#define SCS_ORDER 0 > +#define SCS_SIZE (PAGE_SIZE << SCS_ORDER) > #define GFP_SCS (GFP_KERNEL | __GFP_ZERO) > > /* An illegal pointer value to mark the end of the shadow stack. */ > @@ -33,6 +29,8 @@ > #define task_scs(tsk) (task_thread_info(tsk)->scs_base) > #define task_scs_sp(tsk) (task_thread_info(tsk)->scs_sp) > > +void *scs_alloc(int node); > +void scs_free(void *s); > void scs_init(void); > int scs_prepare(struct task_struct *tsk, int node); > void scs_release(struct task_struct *tsk); > @@ -61,6 +59,8 @@ static inline bool task_scs_end_corrupted(struct task_struct *tsk) > > #else /* CONFIG_SHADOW_CALL_STACK */ > > +static inline void *scs_alloc(int node) { return NULL; } > +static inline void scs_free(void *s) {} > static inline void scs_init(void) {} > static inline void scs_task_reset(struct task_struct *tsk) {} > static inline int scs_prepare(struct task_struct *tsk, int node) { return 0; } > diff --git a/kernel/scs.c b/kernel/scs.c > index 4ff4a7ba0094..25b0dd5aa0e2 100644 > --- a/kernel/scs.c > +++ b/kernel/scs.c > @@ -5,50 +5,94 @@ > * Copyright (C) 2019 Google LLC > */ > > +#include <linux/cpuhotplug.h> > #include <linux/kasan.h> > #include <linux/mm.h> > #include <linux/scs.h> > -#include <linux/slab.h> > +#include <linux/vmalloc.h> > #include <linux/vmstat.h> > > -static struct kmem_cache *scs_cache; > - > static void __scs_account(void *s, int account) > { > - struct page *scs_page = virt_to_page(s); > + struct page *scs_page = vmalloc_to_page(s); > > mod_node_page_state(page_pgdat(scs_page), NR_KERNEL_SCS_KB, > account * (SCS_SIZE / SZ_1K)); > } > > -static void *scs_alloc(int node) > +/* Matches NR_CACHED_STACKS for VMAP_STACK */ > +#define NR_CACHED_SCS 2 > +static DEFINE_PER_CPU(void *, scs_cache[NR_CACHED_SCS]); > + > +void *scs_alloc(int node) > { > - void *s = kmem_cache_alloc_node(scs_cache, GFP_SCS, node); > + int i; > + void *s; > + > + for (i = 0; i < NR_CACHED_SCS; i++) { > + s = this_cpu_xchg(scs_cache[i], NULL); > + if (s) { > + kasan_unpoison_vmalloc(s, SCS_SIZE); > + memset(s, 0, SCS_SIZE); > + goto out; > + } > + } > + > + s = __vmalloc_node_range(SCS_SIZE, 1, VMALLOC_START, VMALLOC_END, > + GFP_SCS, PAGE_KERNEL, 0, node, > + __builtin_return_address(0)); > > if (!s) > return NULL; Sorry I didn't spot this before, but if you put the xchg/vmalloc code into a new __scs_alloc() function then you can drop the label and this becomes: s = __scs_alloc(...); if (!s) return NULL; *__scs_maghic(s) = SCS_ENG_MAGIC; ... With that: Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] scs: switch to vmapped shadow stacks 2020-11-30 11:44 ` Will Deacon @ 2020-11-30 20:03 ` Sami Tolvanen 0 siblings, 0 replies; 10+ messages in thread From: Sami Tolvanen @ 2020-11-30 20:03 UTC (permalink / raw) To: Will Deacon Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, Catalin Marinas, LKML, James Morse, linux-arm-kernel On Mon, Nov 30, 2020 at 3:44 AM Will Deacon <will@kernel.org> wrote: > > On Tue, Nov 24, 2020 at 11:59:39AM -0800, Sami Tolvanen wrote: > > +void *scs_alloc(int node) > > { > > - void *s = kmem_cache_alloc_node(scs_cache, GFP_SCS, node); > > + int i; > > + void *s; > > + > > + for (i = 0; i < NR_CACHED_SCS; i++) { > > + s = this_cpu_xchg(scs_cache[i], NULL); > > + if (s) { > > + kasan_unpoison_vmalloc(s, SCS_SIZE); > > + memset(s, 0, SCS_SIZE); > > + goto out; > > + } > > + } > > + > > + s = __vmalloc_node_range(SCS_SIZE, 1, VMALLOC_START, VMALLOC_END, > > + GFP_SCS, PAGE_KERNEL, 0, node, > > + __builtin_return_address(0)); > > > > if (!s) > > return NULL; > > Sorry I didn't spot this before, but if you put the xchg/vmalloc code > into a new __scs_alloc() function then you can drop the label and this > becomes: > > s = __scs_alloc(...); > if (!s) > return NULL; > > *__scs_maghic(s) = SCS_ENG_MAGIC; > ... Good point, I'll change this in v3. Sami _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI shadow stacks 2020-11-24 19:59 [PATCH v2 0/2] scs: switch to vmapped shadow stacks Sami Tolvanen 2020-11-24 19:59 ` [PATCH v2 1/2] " Sami Tolvanen @ 2020-11-24 19:59 ` Sami Tolvanen 2020-11-30 11:49 ` Will Deacon 1 sibling, 1 reply; 10+ messages in thread From: Sami Tolvanen @ 2020-11-24 19:59 UTC (permalink / raw) To: Will Deacon, Catalin Marinas Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, linux-kernel, James Morse, Sami Tolvanen, linux-arm-kernel Use scs_alloc() to allocate also IRQ and SDEI shadow stacks instead of using statically allocated stacks. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- arch/arm64/kernel/Makefile | 1 - arch/arm64/kernel/entry.S | 6 ++-- arch/arm64/kernel/irq.c | 19 ++++++++++ arch/arm64/kernel/scs.c | 16 --------- arch/arm64/kernel/sdei.c | 71 +++++++++++++++++++++++++++++++------- include/linux/scs.h | 4 --- 6 files changed, 81 insertions(+), 36 deletions(-) delete mode 100644 arch/arm64/kernel/scs.c diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index bbaf0bc4ad60..86364ab6f13f 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -58,7 +58,6 @@ obj-$(CONFIG_CRASH_DUMP) += crash_dump.o obj-$(CONFIG_CRASH_CORE) += crash_core.o obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o -obj-$(CONFIG_SHADOW_CALL_STACK) += scs.o obj-$(CONFIG_ARM64_MTE) += mte.o obj-y += vdso/ probes/ diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index b295fb912b12..5c2ac4b5b2da 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -441,7 +441,7 @@ SYM_CODE_END(__swpan_exit_el0) #ifdef CONFIG_SHADOW_CALL_STACK /* also switch to the irq shadow stack */ - adr_this_cpu scs_sp, irq_shadow_call_stack, x26 + ldr_this_cpu scs_sp, irq_shadow_call_stack_ptr, x26 #endif 9998: @@ -1097,9 +1097,9 @@ SYM_CODE_START(__sdei_asm_handler) #ifdef CONFIG_SHADOW_CALL_STACK /* Use a separate shadow call stack for normal and critical events */ cbnz w4, 3f - adr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_normal, tmp=x6 + ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_normal_ptr, tmp=x6 b 4f -3: adr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_critical, tmp=x6 +3: ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_critical_ptr, tmp=x6 4: #endif diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 9cf2fb87584a..5b7ada9d9559 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -17,6 +17,7 @@ #include <linux/init.h> #include <linux/irqchip.h> #include <linux/kprobes.h> +#include <linux/scs.h> #include <linux/seq_file.h> #include <linux/vmalloc.h> #include <asm/daifflags.h> @@ -27,6 +28,22 @@ DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts); DEFINE_PER_CPU(unsigned long *, irq_stack_ptr); + +DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr); + +#ifdef CONFIG_SHADOW_CALL_STACK +DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr); +#endif + +static void init_irq_scs(void) +{ + int cpu; + + for_each_possible_cpu(cpu) + per_cpu(irq_shadow_call_stack_ptr, cpu) = + scs_alloc(cpu_to_node(cpu)); +} + #ifdef CONFIG_VMAP_STACK static void init_irq_stacks(void) { @@ -54,6 +71,8 @@ static void init_irq_stacks(void) void __init init_IRQ(void) { init_irq_stacks(); + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) + init_irq_scs(); irqchip_init(); if (!handle_arch_irq) panic("No interrupt controller found."); diff --git a/arch/arm64/kernel/scs.c b/arch/arm64/kernel/scs.c deleted file mode 100644 index e8f7ff45dd8f..000000000000 --- a/arch/arm64/kernel/scs.c +++ /dev/null @@ -1,16 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * Shadow Call Stack support. - * - * Copyright (C) 2019 Google LLC - */ - -#include <linux/percpu.h> -#include <linux/scs.h> - -DEFINE_SCS(irq_shadow_call_stack); - -#ifdef CONFIG_ARM_SDE_INTERFACE -DEFINE_SCS(sdei_shadow_call_stack_normal); -DEFINE_SCS(sdei_shadow_call_stack_critical); -#endif diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c index 7689f2031c0c..cbc370d3bb4f 100644 --- a/arch/arm64/kernel/sdei.c +++ b/arch/arm64/kernel/sdei.c @@ -7,6 +7,7 @@ #include <linux/hardirq.h> #include <linux/irqflags.h> #include <linux/sched/task_stack.h> +#include <linux/scs.h> #include <linux/uaccess.h> #include <asm/alternative.h> @@ -37,6 +38,14 @@ DEFINE_PER_CPU(unsigned long *, sdei_stack_normal_ptr); DEFINE_PER_CPU(unsigned long *, sdei_stack_critical_ptr); #endif +DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr); +DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr); + +#ifdef CONFIG_SHADOW_CALL_STACK +DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr); +DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr); +#endif + static void _free_sdei_stack(unsigned long * __percpu *ptr, int cpu) { unsigned long *p; @@ -48,13 +57,31 @@ static void _free_sdei_stack(unsigned long * __percpu *ptr, int cpu) } } +static void _free_sdei_scs(unsigned long * __percpu *ptr, int cpu) +{ + void *s; + + s = per_cpu(*ptr, cpu); + if (s) { + per_cpu(*ptr, cpu) = NULL; + scs_free(s); + } +} + static void free_sdei_stacks(void) { int cpu; for_each_possible_cpu(cpu) { - _free_sdei_stack(&sdei_stack_normal_ptr, cpu); - _free_sdei_stack(&sdei_stack_critical_ptr, cpu); + if (IS_ENABLED(CONFIG_VMAP_STACK)) { + _free_sdei_stack(&sdei_stack_normal_ptr, cpu); + _free_sdei_stack(&sdei_stack_critical_ptr, cpu); + } + + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { + _free_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu); + _free_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu); + } } } @@ -70,18 +97,40 @@ static int _init_sdei_stack(unsigned long * __percpu *ptr, int cpu) return 0; } +static int _init_sdei_scs(unsigned long * __percpu *ptr, int cpu) +{ + void *s; + + s = scs_alloc(cpu_to_node(cpu)); + if (!s) + return -ENOMEM; + per_cpu(*ptr, cpu) = s; + + return 0; +} + static int init_sdei_stacks(void) { int cpu; int err = 0; for_each_possible_cpu(cpu) { - err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); - if (err) - break; - err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); - if (err) - break; + if (IS_ENABLED(CONFIG_VMAP_STACK)) { + err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); + if (err) + break; + err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); + if (err) + break; + } + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { + err = _init_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu); + if (err) + break; + err = _init_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu); + if (err) + break; + } } if (err) @@ -133,10 +182,8 @@ unsigned long sdei_arch_get_entry_point(int conduit) return 0; } - if (IS_ENABLED(CONFIG_VMAP_STACK)) { - if (init_sdei_stacks()) - return 0; - } + if (init_sdei_stacks()) + return 0; sdei_exit_mode = (conduit == SMCCC_CONDUIT_HVC) ? SDEI_EXIT_HVC : SDEI_EXIT_SMC; diff --git a/include/linux/scs.h b/include/linux/scs.h index 2a506c2a16f4..18122d9e17ff 100644 --- a/include/linux/scs.h +++ b/include/linux/scs.h @@ -22,10 +22,6 @@ /* An illegal pointer value to mark the end of the shadow stack. */ #define SCS_END_MAGIC (0x5f6UL + POISON_POINTER_DELTA) -/* Allocate a static per-CPU shadow stack */ -#define DEFINE_SCS(name) \ - DEFINE_PER_CPU(unsigned long [SCS_SIZE/sizeof(long)], name) \ - #define task_scs(tsk) (task_thread_info(tsk)->scs_base) #define task_scs_sp(tsk) (task_thread_info(tsk)->scs_sp) -- 2.29.2.454.gaff20da3a2-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI shadow stacks 2020-11-24 19:59 ` [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI " Sami Tolvanen @ 2020-11-30 11:49 ` Will Deacon 2020-11-30 21:13 ` Sami Tolvanen 0 siblings, 1 reply; 10+ messages in thread From: Will Deacon @ 2020-11-30 11:49 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, Catalin Marinas, linux-kernel, James Morse, linux-arm-kernel On Tue, Nov 24, 2020 at 11:59:40AM -0800, Sami Tolvanen wrote: > Use scs_alloc() to allocate also IRQ and SDEI shadow stacks instead of > using statically allocated stacks. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > arch/arm64/kernel/Makefile | 1 - > arch/arm64/kernel/entry.S | 6 ++-- > arch/arm64/kernel/irq.c | 19 ++++++++++ > arch/arm64/kernel/scs.c | 16 --------- > arch/arm64/kernel/sdei.c | 71 +++++++++++++++++++++++++++++++------- > include/linux/scs.h | 4 --- > 6 files changed, 81 insertions(+), 36 deletions(-) > delete mode 100644 arch/arm64/kernel/scs.c [...] > @@ -70,18 +97,40 @@ static int _init_sdei_stack(unsigned long * __percpu *ptr, int cpu) > return 0; > } > > +static int _init_sdei_scs(unsigned long * __percpu *ptr, int cpu) > +{ > + void *s; > + > + s = scs_alloc(cpu_to_node(cpu)); > + if (!s) > + return -ENOMEM; > + per_cpu(*ptr, cpu) = s; > + > + return 0; > +} > + > static int init_sdei_stacks(void) > { > int cpu; > int err = 0; > > for_each_possible_cpu(cpu) { > - err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > - if (err) > - break; > - err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > - if (err) > - break; > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > + err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > + if (err) > + break; > + err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > + if (err) > + break; > + } > + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { > + err = _init_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu); > + if (err) > + break; > + err = _init_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu); > + if (err) > + break; This looks ok to me, but I think it would be better to follow the same approach as you have for the IRQ stacks and instead have a separate init_sdei_scs() function (similarly for the free() path), which means you can simply the IS_ENABLED() checks too. Either way: Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI shadow stacks 2020-11-30 11:49 ` Will Deacon @ 2020-11-30 21:13 ` Sami Tolvanen 2020-12-01 10:18 ` Will Deacon 0 siblings, 1 reply; 10+ messages in thread From: Sami Tolvanen @ 2020-11-30 21:13 UTC (permalink / raw) To: Will Deacon Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, Catalin Marinas, LKML, James Morse, linux-arm-kernel On Mon, Nov 30, 2020 at 3:49 AM Will Deacon <will@kernel.org> wrote: > > On Tue, Nov 24, 2020 at 11:59:40AM -0800, Sami Tolvanen wrote: > > Use scs_alloc() to allocate also IRQ and SDEI shadow stacks instead of > > using statically allocated stacks. > > > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > --- > > arch/arm64/kernel/Makefile | 1 - > > arch/arm64/kernel/entry.S | 6 ++-- > > arch/arm64/kernel/irq.c | 19 ++++++++++ > > arch/arm64/kernel/scs.c | 16 --------- > > arch/arm64/kernel/sdei.c | 71 +++++++++++++++++++++++++++++++------- > > include/linux/scs.h | 4 --- > > 6 files changed, 81 insertions(+), 36 deletions(-) > > delete mode 100644 arch/arm64/kernel/scs.c > > [...] > > > @@ -70,18 +97,40 @@ static int _init_sdei_stack(unsigned long * __percpu *ptr, int cpu) > > return 0; > > } > > > > +static int _init_sdei_scs(unsigned long * __percpu *ptr, int cpu) > > +{ > > + void *s; > > + > > + s = scs_alloc(cpu_to_node(cpu)); > > + if (!s) > > + return -ENOMEM; > > + per_cpu(*ptr, cpu) = s; > > + > > + return 0; > > +} > > + > > static int init_sdei_stacks(void) > > { > > int cpu; > > int err = 0; > > > > for_each_possible_cpu(cpu) { > > - err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > > - if (err) > > - break; > > - err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > > - if (err) > > - break; > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > + err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > > + if (err) > > + break; > > + err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > > + if (err) > > + break; > > + } > > + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { > > + err = _init_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu); > > + if (err) > > + break; > > + err = _init_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu); > > + if (err) > > + break; > > This looks ok to me, but I think it would be better to follow the same > approach as you have for the IRQ stacks and instead have a separate > init_sdei_scs() function (similarly for the free() path), which means > you can simply the IS_ENABLED() checks too. OK, I can change this in v3. It makes error handling in sdei_arch_get_entry_point() a bit more awkward though. We'll need something like this: if (IS_ENABLED(CONFIG_VMAP_STACK)) { if (init_sdei_stacks()) return 0; } if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { if (init_sdei_scs()) { if (IS_ENABLED(CONFIG_VMAP_STACK)) free_sdei_stacks(); return 0; } } Sami _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI shadow stacks 2020-11-30 21:13 ` Sami Tolvanen @ 2020-12-01 10:18 ` Will Deacon 2020-12-01 10:26 ` Will Deacon 0 siblings, 1 reply; 10+ messages in thread From: Will Deacon @ 2020-12-01 10:18 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, Catalin Marinas, LKML, James Morse, linux-arm-kernel On Mon, Nov 30, 2020 at 01:13:07PM -0800, Sami Tolvanen wrote: > On Mon, Nov 30, 2020 at 3:49 AM Will Deacon <will@kernel.org> wrote: > > > > On Tue, Nov 24, 2020 at 11:59:40AM -0800, Sami Tolvanen wrote: > > > Use scs_alloc() to allocate also IRQ and SDEI shadow stacks instead of > > > using statically allocated stacks. > > > > > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > > --- > > > arch/arm64/kernel/Makefile | 1 - > > > arch/arm64/kernel/entry.S | 6 ++-- > > > arch/arm64/kernel/irq.c | 19 ++++++++++ > > > arch/arm64/kernel/scs.c | 16 --------- > > > arch/arm64/kernel/sdei.c | 71 +++++++++++++++++++++++++++++++------- > > > include/linux/scs.h | 4 --- > > > 6 files changed, 81 insertions(+), 36 deletions(-) > > > delete mode 100644 arch/arm64/kernel/scs.c > > > > [...] > > > > > @@ -70,18 +97,40 @@ static int _init_sdei_stack(unsigned long * __percpu *ptr, int cpu) > > > return 0; > > > } > > > > > > +static int _init_sdei_scs(unsigned long * __percpu *ptr, int cpu) > > > +{ > > > + void *s; > > > + > > > + s = scs_alloc(cpu_to_node(cpu)); > > > + if (!s) > > > + return -ENOMEM; > > > + per_cpu(*ptr, cpu) = s; > > > + > > > + return 0; > > > +} > > > + > > > static int init_sdei_stacks(void) > > > { > > > int cpu; > > > int err = 0; > > > > > > for_each_possible_cpu(cpu) { > > > - err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > > > - if (err) > > > - break; > > > - err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > > > - if (err) > > > - break; > > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > > + err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > > > + if (err) > > > + break; > > > + err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > > > + if (err) > > > + break; > > > + } > > > + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { > > > + err = _init_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu); > > > + if (err) > > > + break; > > > + err = _init_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu); > > > + if (err) > > > + break; > > > > This looks ok to me, but I think it would be better to follow the same > > approach as you have for the IRQ stacks and instead have a separate > > init_sdei_scs() function (similarly for the free() path), which means > > you can simply the IS_ENABLED() checks too. > > OK, I can change this in v3. It makes error handling in > sdei_arch_get_entry_point() a bit more awkward though. We'll need > something like this: > > if (IS_ENABLED(CONFIG_VMAP_STACK)) { > if (init_sdei_stacks()) > return 0; > } > > if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { > if (init_sdei_scs()) { > if (IS_ENABLED(CONFIG_VMAP_STACK)) > free_sdei_stacks(); > return 0; > } Can you push the IS_ENABLED() checks into their respective functions? Then you can do something like: if (init_sdei_stacks()) return 0; if (init_sdei_scs()) goto out_free_stacks; ... out_free_stacks: free_sdei_stacks(); return 0; Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI shadow stacks 2020-12-01 10:18 ` Will Deacon @ 2020-12-01 10:26 ` Will Deacon 0 siblings, 0 replies; 10+ messages in thread From: Will Deacon @ 2020-12-01 10:26 UTC (permalink / raw) To: Sami Tolvanen Cc: Mark Rutland, Kees Cook, Ard Biesheuvel, Catalin Marinas, LKML, James Morse, linux-arm-kernel On Tue, Dec 01, 2020 at 10:18:19AM +0000, Will Deacon wrote: > On Mon, Nov 30, 2020 at 01:13:07PM -0800, Sami Tolvanen wrote: > > On Mon, Nov 30, 2020 at 3:49 AM Will Deacon <will@kernel.org> wrote: > > > On Tue, Nov 24, 2020 at 11:59:40AM -0800, Sami Tolvanen wrote: > > > > for_each_possible_cpu(cpu) { > > > > - err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > > > > - if (err) > > > > - break; > > > > - err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > > > > - if (err) > > > > - break; > > > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > > > + err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > > > > + if (err) > > > > + break; > > > > + err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > > > > + if (err) > > > > + break; > > > > + } > > > > + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { > > > > + err = _init_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu); > > > > + if (err) > > > > + break; > > > > + err = _init_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu); > > > > + if (err) > > > > + break; > > > > > > This looks ok to me, but I think it would be better to follow the same > > > approach as you have for the IRQ stacks and instead have a separate > > > init_sdei_scs() function (similarly for the free() path), which means > > > you can simply the IS_ENABLED() checks too. > > > > OK, I can change this in v3. It makes error handling in > > sdei_arch_get_entry_point() a bit more awkward though. We'll need > > something like this: > > > > if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > if (init_sdei_stacks()) > > return 0; > > } > > > > if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { > > if (init_sdei_scs()) { > > if (IS_ENABLED(CONFIG_VMAP_STACK)) > > free_sdei_stacks(); > > return 0; > > } > > Can you push the IS_ENABLED() checks into their respective functions? > Then you can do something like: > > if (init_sdei_stacks()) > return 0; > > if (init_sdei_scs()) > goto out_free_stacks; > > ... > > out_free_stacks: > free_sdei_stacks(); > return 0; Wait, I see you already posted a v3. Maybe I can just hack this up on top... Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-12-01 10:27 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-24 19:59 [PATCH v2 0/2] scs: switch to vmapped shadow stacks Sami Tolvanen 2020-11-24 19:59 ` [PATCH v2 1/2] " Sami Tolvanen 2020-11-24 22:04 ` Kees Cook 2020-11-30 11:44 ` Will Deacon 2020-11-30 20:03 ` Sami Tolvanen 2020-11-24 19:59 ` [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI " Sami Tolvanen 2020-11-30 11:49 ` Will Deacon 2020-11-30 21:13 ` Sami Tolvanen 2020-12-01 10:18 ` Will Deacon 2020-12-01 10:26 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).