diff mbox series

[v3,2/3] scs: add support for dynamic shadow call stacks

Message ID 20220613134008.3760481-3-ardb@kernel.org (mailing list archive)
State New, archived
Headers show
Series arm64: dynamic shadow call stack support | expand

Commit Message

Ard Biesheuvel June 13, 2022, 1:40 p.m. UTC 
In order to allow arches to use code patching to conditionally emit the
shadow stack pushes and pops, rather than always taking the performance
hit even on CPUs that implement alternatives such as stack pointer
authentication on arm64, add a Kconfig symbol that can be set by the
arch to omit the SCS codegen itself, without otherwise affecting how
support code for SCS and compiler options (for register reservation, for
instance) are emitted.

Also, add a static key and some plumbing to omit the allocation of
shadow call stack for dynamic SCS configurations if SCS is disabled at
runtime.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
 Makefile            |  2 ++
 arch/Kconfig        |  7 +++++++
 include/linux/scs.h | 16 ++++++++++++++++
 kernel/scs.c        | 14 ++++++++++++--
 4 files changed, 37 insertions(+), 2 deletions(-)

Comments

Ard Biesheuvel June 14, 2022, 6:20 a.m. UTC | #1 
On Mon, 13 Jun 2022 at 15:40, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> In order to allow arches to use code patching to conditionally emit the
> shadow stack pushes and pops, rather than always taking the performance
> hit even on CPUs that implement alternatives such as stack pointer
> authentication on arm64, add a Kconfig symbol that can be set by the
> arch to omit the SCS codegen itself, without otherwise affecting how
> support code for SCS and compiler options (for register reservation, for
> instance) are emitted.
>
> Also, add a static key and some plumbing to omit the allocation of
> shadow call stack for dynamic SCS configurations if SCS is disabled at
> runtime.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>

This patch needs the following hunk applied on top to fix a build
error reported by the bots:

--- a/include/linux/scs.h
+++ b/include/linux/scs.h
@@ -57,6 +57,8 @@ DECLARE_STATIC_KEY_TRUE(dynamic_scs_enabled);

 static inline bool scs_is_dynamic(void)
 {
+       if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
+               return false;
        return static_branch_likely(&dynamic_scs_enabled);
 }


> ---
>  Makefile            |  2 ++
>  arch/Kconfig        |  7 +++++++
>  include/linux/scs.h | 16 ++++++++++++++++
>  kernel/scs.c        | 14 ++++++++++++--
>  4 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c43d825a3c4c..806b1dea1218 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -883,8 +883,10 @@ LDFLAGS_vmlinux += --gc-sections
>  endif
>
>  ifdef CONFIG_SHADOW_CALL_STACK
> +ifndef CONFIG_DYNAMIC_SCS
>  CC_FLAGS_SCS   := -fsanitize=shadow-call-stack
>  KBUILD_CFLAGS  += $(CC_FLAGS_SCS)
> +endif
>  export CC_FLAGS_SCS
>  endif
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index fcf9a41a4ef5..a6048d78f05d 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -636,6 +636,13 @@ config SHADOW_CALL_STACK
>           reading and writing arbitrary memory may be able to locate them
>           and hijack control flow by modifying the stacks.
>
> +config DYNAMIC_SCS
> +       bool
> +       help
> +         Set by the arch code if it relies on code patching to insert the
> +         shadow call stack push and pop instructions rather than on the
> +         compiler.
> +
>  config LTO
>         bool
>         help
> diff --git a/include/linux/scs.h b/include/linux/scs.h
> index 18122d9e17ff..c62134d89c7b 100644
> --- a/include/linux/scs.h
> +++ b/include/linux/scs.h
> @@ -53,6 +53,20 @@ static inline bool task_scs_end_corrupted(struct task_struct *tsk)
>         return sz >= SCS_SIZE - 1 || READ_ONCE_NOCHECK(*magic) != SCS_END_MAGIC;
>  }
>
> +DECLARE_STATIC_KEY_TRUE(dynamic_scs_enabled);
> +
> +static inline bool scs_is_dynamic(void)
> +{
> +       return static_branch_likely(&dynamic_scs_enabled);
> +}
> +
> +static inline bool scs_is_enabled(void)
> +{
> +       if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
> +               return true;
> +       return scs_is_dynamic();
> +}
> +
>  #else /* CONFIG_SHADOW_CALL_STACK */
>
>  static inline void *scs_alloc(int node) { return NULL; }
> @@ -62,6 +76,8 @@ static inline void scs_task_reset(struct task_struct *tsk) {}
>  static inline int scs_prepare(struct task_struct *tsk, int node) { return 0; }
>  static inline void scs_release(struct task_struct *tsk) {}
>  static inline bool task_scs_end_corrupted(struct task_struct *tsk) { return false; }
> +static inline bool scs_is_enabled(void) { return false; }
> +static inline bool scs_is_dynamic(void) { return false; }
>
>  #endif /* CONFIG_SHADOW_CALL_STACK */
>
> diff --git a/kernel/scs.c b/kernel/scs.c
> index b7e1b096d906..8826794d2645 100644
> --- a/kernel/scs.c
> +++ b/kernel/scs.c
> @@ -12,6 +12,10 @@
>  #include <linux/vmalloc.h>
>  #include <linux/vmstat.h>
>
> +#ifdef CONFIG_DYNAMIC_SCS
> +DEFINE_STATIC_KEY_TRUE(dynamic_scs_enabled);
> +#endif
> +
>  static void __scs_account(void *s, int account)
>  {
>         struct page *scs_page = vmalloc_to_page(s);
> @@ -101,14 +105,20 @@ static int scs_cleanup(unsigned int cpu)
>
>  void __init scs_init(void)
>  {
> +       if (!scs_is_enabled())
> +               return;
>         cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "scs:scs_cache", NULL,
>                           scs_cleanup);
>  }
>
>  int scs_prepare(struct task_struct *tsk, int node)
>  {
> -       void *s = scs_alloc(node);
> +       void *s;
>
> +       if (!scs_is_enabled())
> +               return 0;
> +
> +       s = scs_alloc(node);
>         if (!s)
>                 return -ENOMEM;
>
> @@ -148,7 +158,7 @@ void scs_release(struct task_struct *tsk)
>  {
>         void *s = task_scs(tsk);
>
> -       if (!s)
> +       if (!scs_is_enabled() || !s)
>                 return;
>
>         WARN(task_scs_end_corrupted(tsk),
> --
> 2.30.2
>
Sami Tolvanen June 15, 2022, 5:12 p.m. UTC | #2 
On Tue, Jun 14, 2022 at 08:20:13AM +0200, Ard Biesheuvel wrote:
> On Mon, 13 Jun 2022 at 15:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > In order to allow arches to use code patching to conditionally emit the
> > shadow stack pushes and pops, rather than always taking the performance
> > hit even on CPUs that implement alternatives such as stack pointer
> > authentication on arm64, add a Kconfig symbol that can be set by the
> > arch to omit the SCS codegen itself, without otherwise affecting how
> > support code for SCS and compiler options (for register reservation, for
> > instance) are emitted.
> >
> > Also, add a static key and some plumbing to omit the allocation of
> > shadow call stack for dynamic SCS configurations if SCS is disabled at
> > runtime.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> This patch needs the following hunk applied on top to fix a build
> error reported by the bots:
> 
> --- a/include/linux/scs.h
> +++ b/include/linux/scs.h
> @@ -57,6 +57,8 @@ DECLARE_STATIC_KEY_TRUE(dynamic_scs_enabled);
> 
>  static inline bool scs_is_dynamic(void)
>  {
> +       if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
> +               return false;
>         return static_branch_likely(&dynamic_scs_enabled);
>  }

With this:

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami
Ard Biesheuvel June 16, 2022, 7:14 a.m. UTC | #3 
On Wed, 15 Jun 2022 at 19:12, Sami Tolvanen <samitolvanen@google.com> wrote:
>
> On Tue, Jun 14, 2022 at 08:20:13AM +0200, Ard Biesheuvel wrote:
> > On Mon, 13 Jun 2022 at 15:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > In order to allow arches to use code patching to conditionally emit the
> > > shadow stack pushes and pops, rather than always taking the performance
> > > hit even on CPUs that implement alternatives such as stack pointer
> > > authentication on arm64, add a Kconfig symbol that can be set by the
> > > arch to omit the SCS codegen itself, without otherwise affecting how
> > > support code for SCS and compiler options (for register reservation, for
> > > instance) are emitted.
> > >
> > > Also, add a static key and some plumbing to omit the allocation of
> > > shadow call stack for dynamic SCS configurations if SCS is disabled at
> > > runtime.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > > Reviewed-by: Kees Cook <keescook@chromium.org>
> >
> > This patch needs the following hunk applied on top to fix a build
> > error reported by the bots:
> >
> > --- a/include/linux/scs.h
> > +++ b/include/linux/scs.h
> > @@ -57,6 +57,8 @@ DECLARE_STATIC_KEY_TRUE(dynamic_scs_enabled);
> >
> >  static inline bool scs_is_dynamic(void)
> >  {
> > +       if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
> > +               return false;
> >         return static_branch_likely(&dynamic_scs_enabled);
> >  }
>
> With this:
>
> Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
>

Thanks. I had spotted that too, and came up with the exact same fix.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index c43d825a3c4c..806b1dea1218 100644
--- a/Makefile
+++ b/Makefile
@@ -883,8 +883,10 @@  LDFLAGS_vmlinux += --gc-sections
 endif
 
 ifdef CONFIG_SHADOW_CALL_STACK
+ifndef CONFIG_DYNAMIC_SCS
 CC_FLAGS_SCS	:= -fsanitize=shadow-call-stack
 KBUILD_CFLAGS	+= $(CC_FLAGS_SCS)
+endif
 export CC_FLAGS_SCS
 endif
 
diff --git a/arch/Kconfig b/arch/Kconfig
index fcf9a41a4ef5..a6048d78f05d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -636,6 +636,13 @@  config SHADOW_CALL_STACK
 	  reading and writing arbitrary memory may be able to locate them
 	  and hijack control flow by modifying the stacks.
 
+config DYNAMIC_SCS
+	bool
+	help
+	  Set by the arch code if it relies on code patching to insert the
+	  shadow call stack push and pop instructions rather than on the
+	  compiler.
+
 config LTO
 	bool
 	help
diff --git a/include/linux/scs.h b/include/linux/scs.h
index 18122d9e17ff..c62134d89c7b 100644
--- a/include/linux/scs.h
+++ b/include/linux/scs.h
@@ -53,6 +53,20 @@  static inline bool task_scs_end_corrupted(struct task_struct *tsk)
 	return sz >= SCS_SIZE - 1 || READ_ONCE_NOCHECK(*magic) != SCS_END_MAGIC;
 }
 
+DECLARE_STATIC_KEY_TRUE(dynamic_scs_enabled);
+
+static inline bool scs_is_dynamic(void)
+{
+	return static_branch_likely(&dynamic_scs_enabled);
+}
+
+static inline bool scs_is_enabled(void)
+{
+	if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
+		return true;
+	return scs_is_dynamic();
+}
+
 #else /* CONFIG_SHADOW_CALL_STACK */
 
 static inline void *scs_alloc(int node) { return NULL; }
@@ -62,6 +76,8 @@  static inline void scs_task_reset(struct task_struct *tsk) {}
 static inline int scs_prepare(struct task_struct *tsk, int node) { return 0; }
 static inline void scs_release(struct task_struct *tsk) {}
 static inline bool task_scs_end_corrupted(struct task_struct *tsk) { return false; }
+static inline bool scs_is_enabled(void) { return false; }
+static inline bool scs_is_dynamic(void) { return false; }
 
 #endif /* CONFIG_SHADOW_CALL_STACK */
 
diff --git a/kernel/scs.c b/kernel/scs.c
index b7e1b096d906..8826794d2645 100644
--- a/kernel/scs.c
+++ b/kernel/scs.c
@@ -12,6 +12,10 @@ 
 #include <linux/vmalloc.h>
 #include <linux/vmstat.h>
 
+#ifdef CONFIG_DYNAMIC_SCS
+DEFINE_STATIC_KEY_TRUE(dynamic_scs_enabled);
+#endif
+
 static void __scs_account(void *s, int account)
 {
 	struct page *scs_page = vmalloc_to_page(s);
@@ -101,14 +105,20 @@  static int scs_cleanup(unsigned int cpu)
 
 void __init scs_init(void)
 {
+	if (!scs_is_enabled())
+		return;
 	cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "scs:scs_cache", NULL,
 			  scs_cleanup);
 }
 
 int scs_prepare(struct task_struct *tsk, int node)
 {
-	void *s = scs_alloc(node);
+	void *s;
 
+	if (!scs_is_enabled())
+		return 0;
+
+	s = scs_alloc(node);
 	if (!s)
 		return -ENOMEM;
 
@@ -148,7 +158,7 @@  void scs_release(struct task_struct *tsk)
 {
 	void *s = task_scs(tsk);
 
-	if (!s)
+	if (!scs_is_enabled() || !s)
 		return;
 
 	WARN(task_scs_end_corrupted(tsk),