Shadow call stacks will be available in GCC >= 12, this patch makes
the corresponding kernel configuration available when compiling
the kernel with the gcc.
Note that the implementation in GCC is slightly different from Clang.
With SCS enabled, functions will only pop x30 once in the epilogue,
like:
str x30, [x18], #8
stp x29, x30, [sp, #-16]!
......
- ldp x29, x30, [sp], #16 //clang
+ ldr x29, [sp], #16 //GCC
ldr x30, [x18, #-8]!
Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=ce09ab17ddd21f73ff2caf6eec3b0ee9b0e1a11e
Signed-off-by: Dan Li <[email protected]>
---
FYI:
This function can be used to test if the shadow call stack works:
//noinline void __noscs scs_test(void)
noinline void scs_test(void)
{
unsigned long * lr = (unsigned long *)__builtin_frame_address(0) + 1;
asm volatile("str xzr, [%0]\n\t": : "r"(lr) : "x30");
}
ffff800008012770 <scs_test>:
ffff800008012770: d503245f bti c
ffff800008012774: d503233f paciasp
ffff800008012778: f800865e str x30, [x18], #8
ffff80000801277c: a9bf7bfd stp x29, x30, [sp, #-16]!
ffff800008012780: 910003fd mov x29, sp
ffff800008012784: 910023a0 add x0, x29, #0x8
ffff800008012788: f900001f str xzr, [x0]
ffff80000801278c: f85f8e5e ldr x30, [x18, #-8]!
ffff800008012790: f84107fd ldr x29, [sp], #16
ffff800008012794: d50323bf autiasp
ffff800008012798: d65f03c0 ret
If SCS protection is enabled, this function will return normally.
If the function has __noscs attribute (scs disabled), it will crash due to 0
address access.
arch/Kconfig | 19 ++++++++++---------
arch/arm64/Kconfig | 2 +-
include/linux/compiler-gcc.h | 4 ++++
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 678a80713b21..c92683362ac2 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -599,21 +599,22 @@ config STACKPROTECTOR_STRONG
config ARCH_SUPPORTS_SHADOW_CALL_STACK
bool
help
- An architecture should select this if it supports Clang's Shadow
- Call Stack and implements runtime support for shadow stack
+ An architecture should select this if it supports the compiler's
+ Shadow Call Stack and implements runtime support for shadow stack
switching.
config SHADOW_CALL_STACK
- bool "Clang Shadow Call Stack"
- depends on CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK
+ bool "Shadow Call Stack"
+ depends on ARCH_SUPPORTS_SHADOW_CALL_STACK
depends on DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER
help
- This option enables Clang's Shadow Call Stack, which uses a
- shadow stack to protect function return addresses from being
- overwritten by an attacker. More information can be found in
- Clang's documentation:
+ This option enables the compiler's Shadow Call Stack, which
+ uses a shadow stack to protect function return addresses from
+ being overwritten by an attacker. More information can be found
+ in the compiler's documentation:
- https://clang.llvm.org/docs/ShadowCallStack.html
+ - Clang (https://clang.llvm.org/docs/ShadowCallStack.html)
+ - GCC (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#Instrumentation-Options)
Note that security guarantees in the kernel differ from the
ones documented for user space. The kernel must store addresses
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 09b885cc4db5..b7145337efae 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1255,7 +1255,7 @@ config HW_PERF_EVENTS
config ARCH_HAS_FILTER_PGPROT
def_bool y
-# Supported by clang >= 7.0
+# Supported by clang >= 7.0 or GCC >= 12.0.0
config CC_HAVE_SHADOW_CALL_STACK
def_bool $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index ccbbd31b3aae..deff5b308470 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -97,6 +97,10 @@
#define KASAN_ABI_VERSION 4
#endif
+#ifdef CONFIG_SHADOW_CALL_STACK
+#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
+#endif
+
#if __has_attribute(__no_sanitize_address__)
#define __no_sanitize_address __attribute__((no_sanitize_address))
#else
--
2.17.1
On Thu, Feb 24, 2022 at 07:24:10PM -0800, Dan Li wrote:
> Shadow call stacks will be available in GCC >= 12, this patch makes
> the corresponding kernel configuration available when compiling
> the kernel with the gcc.
>
> Note that the implementation in GCC is slightly different from Clang.
> With SCS enabled, functions will only pop x30 once in the epilogue,
> like:
>
> str x30, [x18], #8
> stp x29, x30, [sp, #-16]!
> ......
> - ldp x29, x30, [sp], #16 //clang
> + ldr x29, [sp], #16 //GCC
> ldr x30, [x18, #-8]!
>
> Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=ce09ab17ddd21f73ff2caf6eec3b0ee9b0e1a11e
>
> Signed-off-by: Dan Li <[email protected]>
Thanks for the tweaks!
> ---
> FYI:
> This function can be used to test if the shadow call stack works:
> //noinline void __noscs scs_test(void)
> noinline void scs_test(void)
> {
> unsigned long * lr = (unsigned long *)__builtin_frame_address(0) + 1;
>
> asm volatile("str xzr, [%0]\n\t": : "r"(lr) : "x30");
> }
Not a big deal, but just FYI, there's a lot of whitespace trailing the
"}" above...
>
> ffff800008012770 <scs_test>:
> ffff800008012770: d503245f bti c
> ffff800008012774: d503233f paciasp
> ffff800008012778: f800865e str x30, [x18], #8
> ffff80000801277c: a9bf7bfd stp x29, x30, [sp, #-16]!
> ffff800008012780: 910003fd mov x29, sp
> ffff800008012784: 910023a0 add x0, x29, #0x8
> ffff800008012788: f900001f str xzr, [x0]
> ffff80000801278c: f85f8e5e ldr x30, [x18, #-8]!
> ffff800008012790: f84107fd ldr x29, [sp], #16
> ffff800008012794: d50323bf autiasp
> ffff800008012798: d65f03c0 ret
>
> If SCS protection is enabled, this function will return normally.
> If the function has __noscs attribute (scs disabled), it will crash due to 0
> address access.
It would be cool to turn this into an LKDTM test... (see things like the
CFI_FORWARD_PROTO test). I imagine this should be CFI_BACKWARD_SHADOW or
something...
Also, I assume you're using real hardware to test this? It'd be nice to
see if qemu can be convinced to run with the needed features. Whenever
I've tried this it becomes impossibly slow. :)
>
> arch/Kconfig | 19 ++++++++++---------
> arch/arm64/Kconfig | 2 +-
> include/linux/compiler-gcc.h | 4 ++++
> 3 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 678a80713b21..c92683362ac2 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -599,21 +599,22 @@ config STACKPROTECTOR_STRONG
> config ARCH_SUPPORTS_SHADOW_CALL_STACK
> bool
> help
> - An architecture should select this if it supports Clang's Shadow
> - Call Stack and implements runtime support for shadow stack
> + An architecture should select this if it supports the compiler's
> + Shadow Call Stack and implements runtime support for shadow stack
> switching.
>
> config SHADOW_CALL_STACK
> - bool "Clang Shadow Call Stack"
> - depends on CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK
> + bool "Shadow Call Stack"
> + depends on ARCH_SUPPORTS_SHADOW_CALL_STACK
> depends on DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER
> help
> - This option enables Clang's Shadow Call Stack, which uses a
> - shadow stack to protect function return addresses from being
> - overwritten by an attacker. More information can be found in
> - Clang's documentation:
> + This option enables the compiler's Shadow Call Stack, which
> + uses a shadow stack to protect function return addresses from
> + being overwritten by an attacker. More information can be found
> + in the compiler's documentation:
>
> - https://clang.llvm.org/docs/ShadowCallStack.html
> + - Clang (https://clang.llvm.org/docs/ShadowCallStack.html)
> + - GCC (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#Instrumentation-Options)
>
> Note that security guarantees in the kernel differ from the
> ones documented for user space. The kernel must store addresses
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 09b885cc4db5..b7145337efae 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1255,7 +1255,7 @@ config HW_PERF_EVENTS
> config ARCH_HAS_FILTER_PGPROT
> def_bool y
>
> -# Supported by clang >= 7.0
> +# Supported by clang >= 7.0 or GCC >= 12.0.0
> config CC_HAVE_SHADOW_CALL_STACK
> def_bool $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index ccbbd31b3aae..deff5b308470 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -97,6 +97,10 @@
> #define KASAN_ABI_VERSION 4
> #endif
>
> +#ifdef CONFIG_SHADOW_CALL_STACK
> +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
> +#endif
I initially wondered if we need a separate __no_sanitize(STUFF) patch to
make the compiler-clang.h macros easier, but I see there are places
where we do multiple ("address", "hwaddress") and have specialized
macros, so I think this is fine. And since GCC doesn't support
"__has_feature", I think this is the correct location for this.
> +
> #if __has_attribute(__no_sanitize_address__)
> #define __no_sanitize_address __attribute__((no_sanitize_address))
> #else
> --
> 2.17.1
>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, Feb 25, 2022 at 4:24 AM Dan Li <[email protected]> wrote:
>
> + - Clang (https://clang.llvm.org/docs/ShadowCallStack.html)
> + - GCC (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#Instrumentation-Options)
Maybe Clang: and GCC: instead of the parenthesis?
> +#ifdef CONFIG_SHADOW_CALL_STACK
> +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
> +#endif
Since both compilers have it, and I guess the `#ifdef` condition would
work for both, could this be moved into `compiler_types.h` where the
empty `__noscs` definition is, and remove the one from
`compiler-clang.h`?
Cheers,
Miguel
On Fri, Feb 25, 2022 at 09:47:19PM +0100, Miguel Ojeda wrote:
> On Fri, Feb 25, 2022 at 4:24 AM Dan Li <[email protected]> wrote:
> >
> > + - Clang (https://clang.llvm.org/docs/ShadowCallStack.html)
> > + - GCC (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#Instrumentation-Options)
>
> Maybe Clang: and GCC: instead of the parenthesis?
>
> > +#ifdef CONFIG_SHADOW_CALL_STACK
> > +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
> > +#endif
>
> Since both compilers have it, and I guess the `#ifdef` condition would
> work for both, could this be moved into `compiler_types.h` where the
> empty `__noscs` definition is, and remove the one from
> `compiler-clang.h`?
I thought about that too, but I think it's less simple due to the
__has_feature vs CONFIG differences, etc:
https://lore.kernel.org/lkml/202202251243.1E38256F9@keescook/
But maybe __has_feature doesn't really matter here?
--
Kees Cook
On Fri, Feb 25, 2022 at 9:59 PM Kees Cook <[email protected]> wrote:
>
> I thought about that too, but I think it's less simple due to the
> __has_feature vs CONFIG differences, etc:
> https://lore.kernel.org/lkml/202202251243.1E38256F9@keescook/
>
> But maybe __has_feature doesn't really matter here?
I have not tested it, but I was assuming the `CONFIG` could do its
job, since it comes from testing the compiler flag.
If the `__has_feature` is actually restricting it more than that, then
we should probably get the `CONFIG` fixed so that it only gets enabled
when really it is going to be used.
Cheers,
Miguel
On 2/25/22 12:58, Kees Cook wrote:
> On Thu, Feb 24, 2022 at 07:24:10PM -0800, Dan Li wrote:
>> Signed-off-by: Dan Li <[email protected]>
>
> Thanks for the tweaks!
>
>> ---
>> FYI:
>> This function can be used to test if the shadow call stack works:
>> //noinline void __noscs scs_test(void)
>> noinline void scs_test(void)
>> {
>> unsigned long * lr = (unsigned long *)__builtin_frame_address(0) + 1;
>>
>> asm volatile("str xzr, [%0]\n\t": : "r"(lr) : "x30");
>> }
>
> Not a big deal, but just FYI, there's a lot of whitespace trailing the
> "}" above...
>
Ah, sorry for the mistake.
>>
>> If SCS protection is enabled, this function will return normally.
>> If the function has __noscs attribute (scs disabled), it will crash due to 0
>> address access.
>
> It would be cool to turn this into an LKDTM test... (see things like the
> CFI_FORWARD_PROTO test). I imagine this should be CFI_BACKWARD_SHADOW or
> something...
>
OK, I'll add it in the next version.
> Also, I assume you're using real hardware to test this? It'd be nice to
> see if qemu can be convinced to run with the needed features. Whenever
> I've tried this it becomes impossibly slow. :)
>
I also use qemu to test the patch (qemu 6.1.0 with command "-cpu max"),
and can feel the performance drop.
Maybe because my test environment only has simple busybox and ltp,
the feeling of a slow system running is not that strong for me :)
For comparison, I simply tested the difference in kernel boot time
in my test environment:
//run qemu with "-cpu cortex-a57",
[ 1.254481] Run /linuxrc as init process
//run qemu with "-cpu max"
[ 3.566091] Run /linuxrc as init process
>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> index ccbbd31b3aae..deff5b308470 100644
>> --- a/include/linux/compiler-gcc.h
>> +++ b/include/linux/compiler-gcc.h
>> @@ -97,6 +97,10 @@
>> #define KASAN_ABI_VERSION 4
>> #endif
>>
>> +#ifdef CONFIG_SHADOW_CALL_STACK
>> +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
>> +#endif
>
> I initially wondered if we need a separate __no_sanitize(STUFF) patch to
> make the compiler-clang.h macros easier, but I see there are places
> where we do multiple ("address", "hwaddress") and have specialized
> macros, so I think this is fine. And since GCC doesn't support
> "__has_feature", I think this is the correct location for this.
>
As in:
https://lore.kernel.org/all/[email protected]/
I think maybe we could use "#ifdef CONFIG_SHADOW_CALL_STACK"
instead of "#if __has_attribute(__no_sanitize_address__)" here,
then move it to `compiler_types.h`.
From my current test results, __noscs seems to work fine in
clang compilation.
Thanks,
Dan.
On 2/25/22 12:47, Miguel Ojeda wrote:
> On Fri, Feb 25, 2022 at 4:24 AM Dan Li <[email protected]> wrote:
>>
>> + - Clang (https://clang.llvm.org/docs/ShadowCallStack.html)
>> + - GCC (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#Instrumentation-Options)
>
> Maybe Clang: and GCC: instead of the parenthesis?
>
Got it.
>> +#ifdef CONFIG_SHADOW_CALL_STACK
>> +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
>> +#endif
>
> Since both compilers have it, and I guess the `#ifdef` condition would
> work for both, could this be moved into `compiler_types.h` where the
> empty `__noscs` definition is, and remove the one from
> `compiler-clang.h`?
>
In the clang documentation I see __has_feature(shadow_call_stack) is
used to check if -fsanitize=shadow-call-stack is enabled, so I think
maybe it's fine to use "#ifdef CONFIG_SHADOW_CALL_STACK"
instead of "#if __has_attribute(__no_sanitize_address__)" here, then
move it to `compiler_types.h`.
And from the results of my local clang 12 compilation, this doesn't
seem to be a problem.
Link: https://clang.llvm.org/docs/ShadowCallStack.html#has-feature-shadow-call-stack
On Sun, Feb 27, 2022 at 11:37 PM Dan Li <[email protected]> wrote:
>
>
>
> On 2/25/22 12:47, Miguel Ojeda wrote:
> > On Fri, Feb 25, 2022 at 4:24 AM Dan Li <[email protected]> wrote:
> >>
> >> + - Clang (https://clang.llvm.org/docs/ShadowCallStack.html)
> >> + - GCC (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#Instrumentation-Options)
> >
> > Maybe Clang: and GCC: instead of the parenthesis?
> >
> Got it.
>
> >> +#ifdef CONFIG_SHADOW_CALL_STACK
> >> +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
> >> +#endif
> >
> > Since both compilers have it, and I guess the `#ifdef` condition would
> > work for both, could this be moved into `compiler_types.h` where the
> > empty `__noscs` definition is, and remove the one from
> > `compiler-clang.h`?
> >
> In the clang documentation I see __has_feature(shadow_call_stack) is
> used to check if -fsanitize=shadow-call-stack is enabled, so I think
> maybe it's fine to use "#ifdef CONFIG_SHADOW_CALL_STACK"
> instead of "#if __has_attribute(__no_sanitize_address__)" here, then
> move it to `compiler_types.h`.
Or simply add a #define for __noscs to include/linux/compiler-gcc.h
with appropriate guard and leave the existing #ifndef in
include/linux/compiler_types.h as is. I'd prefer that when the
compilers differ in terms of feature detection since it's as explicit
as possible.
>
> And from the results of my local clang 12 compilation, this doesn't
> seem to be a problem.
>
> Link: https://clang.llvm.org/docs/ShadowCallStack.html#has-feature-shadow-call-stack
--
Thanks,
~Nick Desaulniers
On 2/28/22 14:35, Nick Desaulniers wrote:
> On Sun, Feb 27, 2022 at 11:37 PM Dan Li <[email protected]> wrote:
>>>> +#ifdef CONFIG_SHADOW_CALL_STACK
>>>> +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
>>>> +#endif
>>>
>>> Since both compilers have it, and I guess the `#ifdef` condition would
>>> work for both, could this be moved into `compiler_types.h` where the
>>> empty `__noscs` definition is, and remove the one from
>>> `compiler-clang.h`?
>>>
>> In the clang documentation I see __has_feature(shadow_call_stack) is
>> used to check if -fsanitize=shadow-call-stack is enabled, so I think
>> maybe it's fine to use "#ifdef CONFIG_SHADOW_CALL_STACK"
>> instead of "#if __has_attribute(__no_sanitize_address__)" here, then
>> move it to `compiler_types.h`.
>
> Or simply add a #define for __noscs to include/linux/compiler-gcc.h
> with appropriate guard and leave the existing #ifndef in
> include/linux/compiler_types.h as is. I'd prefer that when the
> compilers differ in terms of feature detection since it's as explicit
> as possible.
>
To make sure I understand correctly, that means I should keep
the current patch unchanged right?
Thanks,
Dan.
On Mon, Feb 28, 2022 at 11:35 PM Nick Desaulniers
<[email protected]> wrote:
>
> Or simply add a #define for __noscs to include/linux/compiler-gcc.h
> with appropriate guard and leave the existing #ifndef in
> include/linux/compiler_types.h as is. I'd prefer that when the
> compilers differ in terms of feature detection since it's as explicit
> as possible.
The idea is to avoid differing here to begin with, i.e. to use the
same code for both compilers (only whenever that is possible, of
course), thus having a single `#define` in a single file.
Do you think we will have to change in the future for some reason,
thus needing to split it again?
Cheers,
Miguel
From: Miguel Ojeda
> Sent: 01 March 2022 09:29
>
> On Mon, Feb 28, 2022 at 11:35 PM Nick Desaulniers
> <[email protected]> wrote:
> >
> > Or simply add a #define for __noscs to include/linux/compiler-gcc.h
> > with appropriate guard and leave the existing #ifndef in
> > include/linux/compiler_types.h as is. I'd prefer that when the
> > compilers differ in terms of feature detection since it's as explicit
> > as possible.
>
> The idea is to avoid differing here to begin with, i.e. to use the
> same code for both compilers (only whenever that is possible, of
> course), thus having a single `#define` in a single file.
>
> Do you think we will have to change in the future for some reason,
> thus needing to split it again?
What happens if an out of tree module is compiled with the other compiler?
Surely this is part of the ABI and should be defined for the architecture?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Mon, Feb 28, 2022 at 4:32 PM Dan Li <[email protected]> wrote:
>
>
>
> On 2/28/22 14:35, Nick Desaulniers wrote:
> > On Sun, Feb 27, 2022 at 11:37 PM Dan Li <[email protected]> wrote:
> >>>> +#ifdef CONFIG_SHADOW_CALL_STACK
> >>>> +#define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
> >>>> +#endif
> >>>
> >>> Since both compilers have it, and I guess the `#ifdef` condition would
> >>> work for both, could this be moved into `compiler_types.h` where the
> >>> empty `__noscs` definition is, and remove the one from
> >>> `compiler-clang.h`?
> >>>
> >> In the clang documentation I see __has_feature(shadow_call_stack) is
> >> used to check if -fsanitize=shadow-call-stack is enabled, so I think
> >> maybe it's fine to use "#ifdef CONFIG_SHADOW_CALL_STACK"
> >> instead of "#if __has_attribute(__no_sanitize_address__)" here, then
> >> move it to `compiler_types.h`.
> >
> > Or simply add a #define for __noscs to include/linux/compiler-gcc.h
> > with appropriate guard and leave the existing #ifndef in
> > include/linux/compiler_types.h as is. I'd prefer that when the
> > compilers differ in terms of feature detection since it's as explicit
> > as possible.
> >
>
> To make sure I understand correctly, that means I should keep
> the current patch unchanged right?
Yes.
Reviewed-by: Nick Desaulniers <[email protected]>
--
Thanks,
~Nick Desaulniers