[PATCH v1 2/2] stackprotector: actually use get_random_canary()

Guo Ren guoren at kernel.org
Mon Oct 24 11:47:03 AEDT 2022


On Mon, Oct 24, 2022 at 4:32 AM Jason A. Donenfeld <Jason at zx2c4.com> wrote:
>
> The RNG always mixes in the Linux version extremely early in boot. It
> also always includes a cycle counter, not only during early boot, but
> each and every time it is invoked prior to being fully initialized.
> Together, this means that the use of additional xors inside of the
> various stackprotector.h files is superfluous and over-complicated.
> Instead, we can get exactly the same thing, but better, by just calling
> `get_random_canary()`.
>
> Signed-off-by: Jason A. Donenfeld <Jason at zx2c4.com>
> ---
>  arch/arm/include/asm/stackprotector.h     |  9 +--------
>  arch/arm64/include/asm/stackprotector.h   |  9 +--------
>  arch/csky/include/asm/stackprotector.h    | 10 +---------
>  arch/mips/include/asm/stackprotector.h    |  9 +--------
>  arch/powerpc/include/asm/stackprotector.h | 10 +---------
>  arch/riscv/include/asm/stackprotector.h   | 10 +---------
>  arch/sh/include/asm/stackprotector.h      | 10 +---------
>  arch/x86/include/asm/stackprotector.h     | 14 +-------------
>  arch/xtensa/include/asm/stackprotector.h  |  7 +------
>  9 files changed, 9 insertions(+), 79 deletions(-)
>
> diff --git a/arch/arm/include/asm/stackprotector.h b/arch/arm/include/asm/stackprotector.h
> index 088d03161be5..0bd4979759f1 100644
> --- a/arch/arm/include/asm/stackprotector.h
> +++ b/arch/arm/include/asm/stackprotector.h
> @@ -15,9 +15,6 @@
>  #ifndef _ASM_STACKPROTECTOR_H
>  #define _ASM_STACKPROTECTOR_H 1
>
> -#include <linux/random.h>
> -#include <linux/version.h>
> -
>  #include <asm/thread_info.h>
>
>  extern unsigned long __stack_chk_guard;
> @@ -30,11 +27,7 @@ extern unsigned long __stack_chk_guard;
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> -       unsigned long canary;
> -
> -       /* Try to get a semi random initial value. */
> -       get_random_bytes(&canary, sizeof(canary));
> -       canary ^= LINUX_VERSION_CODE;
> +       unsigned long canary = get_random_canary();
>
>         current->stack_canary = canary;
>  #ifndef CONFIG_STACKPROTECTOR_PER_TASK
> diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
> index 33f1bb453150..ae3ad80f51fe 100644
> --- a/arch/arm64/include/asm/stackprotector.h
> +++ b/arch/arm64/include/asm/stackprotector.h
> @@ -13,8 +13,6 @@
>  #ifndef __ASM_STACKPROTECTOR_H
>  #define __ASM_STACKPROTECTOR_H
>
> -#include <linux/random.h>
> -#include <linux/version.h>
>  #include <asm/pointer_auth.h>
>
>  extern unsigned long __stack_chk_guard;
> @@ -28,12 +26,7 @@ extern unsigned long __stack_chk_guard;
>  static __always_inline void boot_init_stack_canary(void)
>  {
>  #if defined(CONFIG_STACKPROTECTOR)
> -       unsigned long canary;
> -
> -       /* Try to get a semi random initial value. */
> -       get_random_bytes(&canary, sizeof(canary));
> -       canary ^= LINUX_VERSION_CODE;
> -       canary &= CANARY_MASK;
> +       unsigned long canary = get_random_canary();
>
>         current->stack_canary = canary;
>         if (!IS_ENABLED(CONFIG_STACKPROTECTOR_PER_TASK))
> diff --git a/arch/csky/include/asm/stackprotector.h b/arch/csky/include/asm/stackprotector.h
> index d7cd4e51edd9..d23747447166 100644
> --- a/arch/csky/include/asm/stackprotector.h
> +++ b/arch/csky/include/asm/stackprotector.h
> @@ -2,9 +2,6 @@
>  #ifndef _ASM_STACKPROTECTOR_H
>  #define _ASM_STACKPROTECTOR_H 1
>
> -#include <linux/random.h>
> -#include <linux/version.h>
> -
>  extern unsigned long __stack_chk_guard;
>
>  /*
> @@ -15,12 +12,7 @@ extern unsigned long __stack_chk_guard;
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> -       unsigned long canary;
> -
> -       /* Try to get a semi random initial value. */
> -       get_random_bytes(&canary, sizeof(canary));
> -       canary ^= LINUX_VERSION_CODE;
> -       canary &= CANARY_MASK;
> +       unsigned long canary = get_random_canary();
Acked-by: Guo Ren <guoren at kernel.org> #csky part

>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
> diff --git a/arch/mips/include/asm/stackprotector.h b/arch/mips/include/asm/stackprotector.h
> index 68d4be9e1254..518c192ad982 100644
> --- a/arch/mips/include/asm/stackprotector.h
> +++ b/arch/mips/include/asm/stackprotector.h
> @@ -15,9 +15,6 @@
>  #ifndef _ASM_STACKPROTECTOR_H
>  #define _ASM_STACKPROTECTOR_H 1
>
> -#include <linux/random.h>
> -#include <linux/version.h>
> -
>  extern unsigned long __stack_chk_guard;
>
>  /*
> @@ -28,11 +25,7 @@ extern unsigned long __stack_chk_guard;
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> -       unsigned long canary;
> -
> -       /* Try to get a semi random initial value. */
> -       get_random_bytes(&canary, sizeof(canary));
> -       canary ^= LINUX_VERSION_CODE;
> +       unsigned long canary = get_random_canary();
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
> diff --git a/arch/powerpc/include/asm/stackprotector.h b/arch/powerpc/include/asm/stackprotector.h
> index 1c8460e23583..283c34647856 100644
> --- a/arch/powerpc/include/asm/stackprotector.h
> +++ b/arch/powerpc/include/asm/stackprotector.h
> @@ -7,8 +7,6 @@
>  #ifndef _ASM_STACKPROTECTOR_H
>  #define _ASM_STACKPROTECTOR_H
>
> -#include <linux/random.h>
> -#include <linux/version.h>
>  #include <asm/reg.h>
>  #include <asm/current.h>
>  #include <asm/paca.h>
> @@ -21,13 +19,7 @@
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> -       unsigned long canary;
> -
> -       /* Try to get a semi random initial value. */
> -       canary = get_random_canary();
> -       canary ^= mftb();
> -       canary ^= LINUX_VERSION_CODE;
> -       canary &= CANARY_MASK;
> +       unsigned long canary = get_random_canary();
>
>         current->stack_canary = canary;
>  #ifdef CONFIG_PPC64
> diff --git a/arch/riscv/include/asm/stackprotector.h b/arch/riscv/include/asm/stackprotector.h
> index 09093af46565..43895b90fe3f 100644
> --- a/arch/riscv/include/asm/stackprotector.h
> +++ b/arch/riscv/include/asm/stackprotector.h
> @@ -3,9 +3,6 @@
>  #ifndef _ASM_RISCV_STACKPROTECTOR_H
>  #define _ASM_RISCV_STACKPROTECTOR_H
>
> -#include <linux/random.h>
> -#include <linux/version.h>
> -
>  extern unsigned long __stack_chk_guard;
>
>  /*
> @@ -16,12 +13,7 @@ extern unsigned long __stack_chk_guard;
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> -       unsigned long canary;
> -
> -       /* Try to get a semi random initial value. */
> -       get_random_bytes(&canary, sizeof(canary));
> -       canary ^= LINUX_VERSION_CODE;
> -       canary &= CANARY_MASK;
> +       unsigned long canary = get_random_canary();
>
>         current->stack_canary = canary;
>         if (!IS_ENABLED(CONFIG_STACKPROTECTOR_PER_TASK))
> diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
> index 35616841d0a1..665dafac376f 100644
> --- a/arch/sh/include/asm/stackprotector.h
> +++ b/arch/sh/include/asm/stackprotector.h
> @@ -2,9 +2,6 @@
>  #ifndef __ASM_SH_STACKPROTECTOR_H
>  #define __ASM_SH_STACKPROTECTOR_H
>
> -#include <linux/random.h>
> -#include <linux/version.h>
> -
>  extern unsigned long __stack_chk_guard;
>
>  /*
> @@ -15,12 +12,7 @@ extern unsigned long __stack_chk_guard;
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> -       unsigned long canary;
> -
> -       /* Try to get a semi random initial value. */
> -       get_random_bytes(&canary, sizeof(canary));
> -       canary ^= LINUX_VERSION_CODE;
> -       canary &= CANARY_MASK;
> +       unsigned long canary = get_random_canary();
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
> diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
> index 24a8d6c4fb18..00473a650f51 100644
> --- a/arch/x86/include/asm/stackprotector.h
> +++ b/arch/x86/include/asm/stackprotector.h
> @@ -34,7 +34,6 @@
>  #include <asm/percpu.h>
>  #include <asm/desc.h>
>
> -#include <linux/random.h>
>  #include <linux/sched.h>
>
>  /*
> @@ -50,22 +49,11 @@
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> -       u64 canary;
> -       u64 tsc;
> +       unsigned long canary = get_random_canary();
>
>  #ifdef CONFIG_X86_64
>         BUILD_BUG_ON(offsetof(struct fixed_percpu_data, stack_canary) != 40);
>  #endif
> -       /*
> -        * We both use the random pool and the current TSC as a source
> -        * of randomness. The TSC only matters for very early init,
> -        * there it already has some randomness on most systems. Later
> -        * on during the bootup the random pool has true entropy too.
> -        */
> -       get_random_bytes(&canary, sizeof(canary));
> -       tsc = rdtsc();
> -       canary += tsc + (tsc << 32UL);
> -       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>  #ifdef CONFIG_X86_64
> diff --git a/arch/xtensa/include/asm/stackprotector.h b/arch/xtensa/include/asm/stackprotector.h
> index e368f94fd2af..e1e318a0c98a 100644
> --- a/arch/xtensa/include/asm/stackprotector.h
> +++ b/arch/xtensa/include/asm/stackprotector.h
> @@ -14,7 +14,6 @@
>  #ifndef _ASM_STACKPROTECTOR_H
>  #define _ASM_STACKPROTECTOR_H 1
>
> -#include <linux/random.h>
>  #include <linux/version.h>
>
>  extern unsigned long __stack_chk_guard;
> @@ -27,11 +26,7 @@ extern unsigned long __stack_chk_guard;
>   */
>  static __always_inline void boot_init_stack_canary(void)
>  {
> -       unsigned long canary;
> -
> -       /* Try to get a semi random initial value. */
> -       get_random_bytes(&canary, sizeof(canary));
> -       canary ^= LINUX_VERSION_CODE;
> +       unsigned long canary = get_random_canary();
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
> --
> 2.38.1
>


-- 
Best Regards
 Guo Ren


More information about the Linuxppc-dev mailing list