| 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: Thu, 19 Nov 2020 14:41:34 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Andre Przywara <andre.przywara@....com>
Cc: Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Russell King <linux@...linux.org.uk>,
Marc Zyngier <maz@...nel.org>, "Theodore Ts'o" <tytso@....edu>,
Sudeep Holla <sudeep.holla@....com>,
Mark Rutland <mark.rutland@....com>,
Mark Brown <broonie@...nel.org>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Linus Walleij <linus.walleij@...aro.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
kvmarm <kvmarm@...ts.cs.columbia.edu>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 4/5] arm64: Add support for SMCCC TRNG entropy source
On Fri, 13 Nov 2020 at 19:24, Andre Przywara <andre.przywara@....com> wrote:
>
> The ARM architected TRNG firmware interface, described in ARM spec
> DEN0098, defines an ARM SMCCC based interface to a true random number
> generator, provided by firmware.
> This can be discovered via the SMCCC >=v1.1 interface, and provides
> up to 192 bits of entropy per call.
>
> Hook this SMC call into arm64's arch_get_random_*() implementation,
> coming to the rescue when the CPU does not implement the ARM v8.5 RNG
> system registers.
>
> For the detection, we piggy back on the PSCI/SMCCC discovery (which gives
> us the conduit to use (hvc/smc)), then try to call the
> ARM_SMCCC_TRNG_VERSION function, which returns -1 if this interface is
> not implemented.
>
> Signed-off-by: Andre Przywara <andre.przywara@....com>
> ---
> arch/arm64/include/asm/archrandom.h | 69 ++++++++++++++++++++++++-----
> 1 file changed, 58 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> index abe07c21da8e..fe34bfd30caa 100644
> --- a/arch/arm64/include/asm/archrandom.h
> +++ b/arch/arm64/include/asm/archrandom.h
> @@ -4,13 +4,24 @@
>
> #ifdef CONFIG_ARCH_RANDOM
>
> +#include <linux/arm-smccc.h>
> #include <linux/bug.h>
> #include <linux/kernel.h>
> #include <asm/cpufeature.h>
>
> +#define ARM_SMCCC_TRNG_MIN_VERSION 0x10000UL
> +
> +extern bool smccc_trng_available;
> +
> static inline bool __init smccc_probe_trng(void)
> {
> - return false;
> + struct arm_smccc_res res;
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
> + if ((s32)res.a0 < 0)
> + return false;
> +
> + return res.a0 >= ARM_SMCCC_TRNG_MIN_VERSION;
> }
>
> static inline bool __arm64_rndr(unsigned long *v)
> @@ -43,26 +54,52 @@ static inline bool __must_check arch_get_random_int(unsigned int *v)
>
> static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
> {
> + struct arm_smccc_res res;
> +
> /*
> * Only support the generic interface after we have detected
> * the system wide capability, avoiding complexity with the
> * cpufeature code and with potential scheduling between CPUs
> * with and without the feature.
> */
> - if (!cpus_have_const_cap(ARM64_HAS_RNG))
> - return false;
> + if (cpus_have_const_cap(ARM64_HAS_RNG))
> + return __arm64_rndr(v);
>
> - return __arm64_rndr(v);
> -}
> + if (smccc_trng_available) {
> + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
> + if ((int)res.a0 < 0)
> + return false;
>
> + *v = res.a3;
> + return true;
> + }
> +
> + return false;
> +}
>
I think we should be more rigorous here in how we map the concepts of
random seeds and random numbers onto the various sources.
First of all, assuming my patch dropping the call to
arch_get_random_seed_long() from add_interrupt_randomness() gets
accepted, we should switch to RNDRRS here, and implement the non-seed
variants using RNDR.
However, this is still semantically inaccurate: RNDRRS does not return
a random *seed*, it returns a number drawn from a freshly seeded
pseudo-random sequence. This means that the TRNG interface, if
implemented, is a better choice, and so we should try it first. Note
that on platforms that don't implement both, only one of these will be
available in the first place. But on platforms that *do* implement
both, the firmware interface may actually be less wasteful in terms of
resources: the TRNG interface returns every bit drawn from the
underlying entropy source, whereas RNDRRS uses ~500 bits of entropy to
reseed a DRBG that gets used only once to draw a single 64-bit number.
And the cost of the SMCCC call in terms of CPU time is charged to the
caller, which is appropriate here.
Then, I don't think we should ever return false without even trying if
RNDRRS is available if the SMCCC invocation fails.
Something like this perhaps?
if (smccc_trng_available) {
arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
if ((int)res.a0 >= 0) {
*v = res.a3;
return true;
}
}
if (cpus_have_const_cap(ARM64_HAS_RNG))
return __arm64_rndrrs(v);
return false;
(and something similar 2x below)
> static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
> {
> + struct arm_smccc_res res;
> unsigned long val;
> - bool ok = arch_get_random_seed_long(&val);
>
> - *v = val;
> - return ok;
> + if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> + if (arch_get_random_seed_long(&val)) {
> + *v = val;
> + return true;
> + }
> + return false;
> + }
> +
> + if (smccc_trng_available) {
> + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 32, &res);
> + if ((int)res.a0 < 0)
> + return false;
> +
> + *v = res.a3 & GENMASK(31, 0);
> + return true;
> + }
> +
> + return false;
> }
>
> static inline bool __init __early_cpu_has_rndr(void)
> @@ -77,10 +114,20 @@ arch_get_random_seed_long_early(unsigned long *v)
> {
> WARN_ON(system_state != SYSTEM_BOOTING);
>
> - if (!__early_cpu_has_rndr())
> - return false;
> + if (__early_cpu_has_rndr())
> + return __arm64_rndr(v);
> +
> + if (smccc_trng_available) {
> + struct arm_smccc_res res;
>
> - return __arm64_rndr(v);
> + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
> + if ((int)res.a0 >= 0) {
> + *v = res.a3;
> + return true;
> + }
> + }
> +
> + return false;
> }
> #define arch_get_random_seed_long_early arch_get_random_seed_long_early
>
> --
> 2.17.1
>
Powered by blists - more mailing lists