| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 24 Oct 2022 08:47:03 +0800
From: Guo Ren <guoren@...nel.org>
To: "Jason A. Donenfeld" <Jason@...c4.com>
Cc: linux-kernel@...r.kernel.org, Albert Ou <aou@...s.berkeley.edu>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Borislav Petkov <bp@...en8.de>,
Catalin Marinas <catalin.marinas@....com>,
Chris Zankel <chris@...kel.net>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"H . Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
Juergen Gross <jgross@...e.com>,
Max Filippov <jcmvbkbc@...il.com>,
Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>,
Palmer Dabbelt <palmer@...belt.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Rich Felker <dalias@...c.org>,
Russell King <linux@...linux.org.uk>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Thomas Gleixner <tglx@...utronix.de>,
Will Deacon <will@...nel.org>,
Yoshinori Sato <ysato@...rs.sourceforge.jp>,
linux-arm-kernel@...ts.infradead.org, linux-csky@...r.kernel.org,
linux-mips@...r.kernel.org, linux-riscv@...ts.infradead.org,
linux-sh@...r.kernel.org, linux-xtensa@...ux-xtensa.org,
linuxppc-dev@...ts.ozlabs.org, x86@...nel.org
Subject: Re: [PATCH v1 2/2] stackprotector: actually use get_random_canary()
On Mon, Oct 24, 2022 at 4:32 AM Jason A. Donenfeld <Jason@...c4.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@...c4.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@...nel.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
Powered by blists - more mailing lists