| 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
| ||
|
Message-ID: <Y2ESilMCF9eeffW6@FVFF77S0Q05N>
Date: Tue, 1 Nov 2022 12:36:07 +0000
From: Mark Rutland <mark.rutland@....com>
To: "Jason A. Donenfeld" <Jason@...c4.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Will Deacon <will@...nel.org>,
Ard Biesheuvel <ardb@...nel.org>,
Jean-Philippe Brucker <jean-philippe@...aro.org>
Subject: Re: [PATCH v5] random: remove early archrandom abstraction
Hi Jason,
Sorry for joining this late...
On Tue, Nov 01, 2022 at 01:25:28PM +0100, Jason A. Donenfeld wrote:
> The arch_get_random*_early() abstraction is not completely useful and
> adds complexity, because it's not a given that there will be no calls to
> arch_get_random*() between random_init_early(), which uses
> arch_get_random*_early(), and init_cpu_features(). During that gap,
> crng_reseed() might be called, which uses arch_get_random*(), since it's
> mostly not init code.
The original rationale for arch_get_random*_early() was just to seed the RNG
more robustly rather than to feed every possible arch_get_random() call made
early in the boot flow, and the rationale for having a separate functions was
that it was trivial to see by inspection that it was (only) called in the
expected places.
I'm not wedded to arch_get_random*_early() specifically, but I do think that
having arch_get_random() behave differently depending on which phase of boot
we're in has more scope for error than having a separate call of some sort.
Other than removing the lines below, what chages is this going to permit?
Thanks,
Mark.
> Instead we can test whether we're in the early phase in
> arch_get_random*() itself, and in doing so avoid all ambiguity about
> where we are. Fortunately, the only architecture that currently
> implements arch_get_random*_early() also has an alternatives-based cpu
> feature system, one flag of which determines whether the other flags
> have been initialized. This makes it possible to do the early check with
> zero cost once the system is initialized.
>
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Will Deacon <will@...nel.org>
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Cc: Jean-Philippe Brucker <jean-philippe@...aro.org>
> Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
> ---
> Changes v4->v5:
> - Use this_cpu_has_cap() to make codegen for the cold path smaller.
> - Add manual unlikely() to make the coldpath that's patched out even
> further away.
>
> Changes v3->v4:
> - Use !preemptible() rather than early_boot_irqs_disabled.
>
> arch/arm64/include/asm/archrandom.h | 48 ++++++-----------------------
> drivers/char/random.c | 4 +--
> include/linux/random.h | 20 ------------
> 3 files changed, 12 insertions(+), 60 deletions(-)
>
> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> index 109e2a4454be..2f5f3da34782 100644
> --- a/arch/arm64/include/asm/archrandom.h
> +++ b/arch/arm64/include/asm/archrandom.h
> @@ -5,6 +5,7 @@
> #include <linux/arm-smccc.h>
> #include <linux/bug.h>
> #include <linux/kernel.h>
> +#include <linux/irqflags.h>
> #include <asm/cpufeature.h>
>
> #define ARM_SMCCC_TRNG_MIN_VERSION 0x10000UL
> @@ -58,6 +59,13 @@ static inline bool __arm64_rndrrs(unsigned long *v)
> return ok;
> }
>
> +static __always_inline bool __cpu_has_rng(void)
> +{
> + if (unlikely(!system_capabilities_finalized() && !preemptible()))
> + return this_cpu_has_cap(ARM64_HAS_RNG);
> + return cpus_have_const_cap(ARM64_HAS_RNG);
> +}
> +
> static inline size_t __must_check arch_get_random_longs(unsigned long *v, size_t max_longs)
> {
> /*
> @@ -66,7 +74,7 @@ static inline size_t __must_check arch_get_random_longs(unsigned long *v, size_t
> * cpufeature code and with potential scheduling between CPUs
> * with and without the feature.
> */
> - if (max_longs && cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v))
> + if (max_longs && __cpu_has_rng() && __arm64_rndr(v))
> return 1;
> return 0;
> }
> @@ -108,7 +116,7 @@ static inline size_t __must_check arch_get_random_seed_longs(unsigned long *v, s
> * reseeded after each invocation. This is not a 100% fit but good
> * enough to implement this API if no other entropy source exists.
> */
> - if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndrrs(v))
> + if (__cpu_has_rng() && __arm64_rndrrs(v))
> return 1;
>
> return 0;
> @@ -121,40 +129,4 @@ static inline bool __init __early_cpu_has_rndr(void)
> return (ftr >> ID_AA64ISAR0_EL1_RNDR_SHIFT) & 0xf;
> }
>
> -static inline size_t __init __must_check
> -arch_get_random_seed_longs_early(unsigned long *v, size_t max_longs)
> -{
> - WARN_ON(system_state != SYSTEM_BOOTING);
> -
> - if (!max_longs)
> - return 0;
> -
> - if (smccc_trng_available) {
> - struct arm_smccc_res res;
> -
> - max_longs = min_t(size_t, 3, max_longs);
> - arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, max_longs * 64, &res);
> - if ((int)res.a0 >= 0) {
> - switch (max_longs) {
> - case 3:
> - *v++ = res.a1;
> - fallthrough;
> - case 2:
> - *v++ = res.a2;
> - fallthrough;
> - case 1:
> - *v++ = res.a3;
> - break;
> - }
> - return max_longs;
> - }
> - }
> -
> - if (__early_cpu_has_rndr() && __arm64_rndr(v))
> - return 1;
> -
> - return 0;
> -}
> -#define arch_get_random_seed_longs_early arch_get_random_seed_longs_early
> -
> #endif /* _ASM_ARCHRANDOM_H */
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 9d016ac181fa..5da120f10a67 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -813,13 +813,13 @@ void __init random_init_early(const char *command_line)
> #endif
>
> for (i = 0, arch_bits = sizeof(entropy) * 8; i < ARRAY_SIZE(entropy);) {
> - longs = arch_get_random_seed_longs_early(entropy, ARRAY_SIZE(entropy) - i);
> + longs = arch_get_random_seed_longs(entropy, ARRAY_SIZE(entropy) - i);
> if (longs) {
> _mix_pool_bytes(entropy, sizeof(*entropy) * longs);
> i += longs;
> continue;
> }
> - longs = arch_get_random_longs_early(entropy, ARRAY_SIZE(entropy) - i);
> + longs = arch_get_random_longs(entropy, ARRAY_SIZE(entropy) - i);
> if (longs) {
> _mix_pool_bytes(entropy, sizeof(*entropy) * longs);
> i += longs;
> diff --git a/include/linux/random.h b/include/linux/random.h
> index 182780cafd45..2bdd3add3400 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -153,26 +153,6 @@ declare_get_random_var_wait(long, unsigned long)
>
> #include <asm/archrandom.h>
>
> -/*
> - * Called from the boot CPU during startup; not valid to call once
> - * secondary CPUs are up and preemption is possible.
> - */
> -#ifndef arch_get_random_seed_longs_early
> -static inline size_t __init arch_get_random_seed_longs_early(unsigned long *v, size_t max_longs)
> -{
> - WARN_ON(system_state != SYSTEM_BOOTING);
> - return arch_get_random_seed_longs(v, max_longs);
> -}
> -#endif
> -
> -#ifndef arch_get_random_longs_early
> -static inline bool __init arch_get_random_longs_early(unsigned long *v, size_t max_longs)
> -{
> - WARN_ON(system_state != SYSTEM_BOOTING);
> - return arch_get_random_longs(v, max_longs);
> -}
> -#endif
> -
> #ifdef CONFIG_SMP
> int random_prepare_cpu(unsigned int cpu);
> int random_online_cpu(unsigned int cpu);
> --
> 2.38.1
>
Powered by blists - more mailing lists