Hi Jason,
On 7/6/22 17:00, Jason A. Donenfeld wrote:
> The decision of whether or not to trust RDRAND is controlled by the
> "random.trust_cpu" boot time parameter or the CONFIG_RANDOM_TRUST_CPU
> compile time default. The "nordrand" flag was added during the early
> days of RDRAND, when there were worries that merely using its values
> could compromise the RNG. However, these days, RDRAND values are not
> used directly but always go through the RNG's hash function, making
> "nordrand" no longer useful.
>
> Rather, the correct switch is "random.trust_cpu", which not only handles
> the relevant trust issue directly, but also is general to multiple CPU
> types, not just x86.
>
> However, x86 RDRAND does have a history of being occassionally
occasionally
> problematic. Prior, when the kernel would notice something strange, it'd
> warn in dmesg and suggest enabling "nordrand". We can improve on that by
> making the test a little bit better and then taking the step of
> automatically disabling "nordrand" if we detect it's problematic.
enabling
? I.e., disabling RDRAND and RDSEED.
>
> Also extend the basic sanity test to RDSEED in addition to RDRAND, and
> disable both if either one fails.
>
> Cc: [email protected]
> Cc: Theodore Ts'o <[email protected]>
> Suggested-by: H. Peter Anvin <[email protected]>
> Suggested-by: Borislav Petkov <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>
> ---
> This patch builds on top of v4 of "random: remove CONFIG_ARCH_RANDOM":
> https://lore.kernel.org/lkml/[email protected]/
>
> .../admin-guide/kernel-parameters.txt | 5 --
> arch/x86/kernel/cpu/amd.c | 2 +-
> arch/x86/kernel/cpu/rdrand.c | 73 +++++++++----------
> 3 files changed, 36 insertions(+), 44 deletions(-)
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 0c0b09796ced..216fc2f53cbe 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -808,7 +808,7 @@ static void clear_rdrand_cpuid_bit(struct cpuinfo_x86 *c)
> return;
>
> /*
> - * The nordrand option can clear X86_FEATURE_RDRAND, so check for
> + * The self test can clear X86_FEATURE_RDRAND, so check for
Preferably: self-test
> * RDRAND support using the CPUID function directly.
> */
> if (!(cpuid_ecx(1) & BIT(30)) || rdrand_force)
--
~Randy
Hi Randy,
On Thu, Jul 7, 2022 at 2:12 AM Randy Dunlap <[email protected]> wrote:
> > However, x86 RDRAND does have a history of being occassionally
>
> occasionally
>
> > automatically disabling "nordrand" if we detect it's problematic.
> ? I.e., disabling RDRAND and RDSEED.
> > - * The nordrand option can clear X86_FEATURE_RDRAND, so check for
> > + * The self test can clear X86_FEATURE_RDRAND, so check for
>
> Preferably: self-test
Thanks. Fixing all of those.
Jason
The decision of whether or not to trust RDRAND is controlled by the
"random.trust_cpu" boot time parameter or the CONFIG_RANDOM_TRUST_CPU
compile time default. The "nordrand" flag was added during the early
days of RDRAND, when there were worries that merely using its values
could compromise the RNG. However, these days, RDRAND values are not
used directly but always go through the RNG's hash function, making
"nordrand" no longer useful.
Rather, the correct switch is "random.trust_cpu", which not only handles
the relevant trust issue directly, but also is general to multiple CPU
types, not just x86.
However, x86 RDRAND does have a history of being occasionally
problematic. Prior, when the kernel would notice something strange, it'd
warn in dmesg and suggest enabling "nordrand". We can improve on that by
making the test a little bit better and then taking the step of
automatically disabling RDRAND if we detect it's problematic.
Also extend the basic sanity test to RDSEED in addition to RDRAND, and
disable both if either one fails.
Cc: [email protected]
Cc: Theodore Ts'o <[email protected]>
Suggested-by: H. Peter Anvin <[email protected]>
Suggested-by: Borislav Petkov <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
This is a tip-ish commit, but it relies on the CONFIG_ARCH_RANDOM commit
in the random tree, so I'll take this through the random tree to avoid
conflicts.
Changes v1->v2:
- [Borislav] Reformat into tip-style commit to be taken through random
tree.
- [Randy] Fix typos.
.../admin-guide/kernel-parameters.txt | 5 --
arch/x86/kernel/cpu/amd.c | 2 +-
arch/x86/kernel/cpu/rdrand.c | 73 +++++++++----------
3 files changed, 36 insertions(+), 44 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2522b11e593f..a1dc4dbf74f6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3733,11 +3733,6 @@
noreplace-smp [X86-32,SMP] Don't replace SMP instructions
with UP alternatives
- nordrand [X86] Disable kernel use of the RDRAND and
- RDSEED instructions even if they are supported
- by the processor. RDRAND and RDSEED are still
- available to user space applications.
-
noresume [SWSUSP] Disables resume and restores original swap
space.
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 0c0b09796ced..8baf312e030a 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -808,7 +808,7 @@ static void clear_rdrand_cpuid_bit(struct cpuinfo_x86 *c)
return;
/*
- * The nordrand option can clear X86_FEATURE_RDRAND, so check for
+ * The self-test can clear X86_FEATURE_RDRAND, so check for
* RDRAND support using the CPUID function directly.
*/
if (!(cpuid_ecx(1) & BIT(30)) || rdrand_force)
diff --git a/arch/x86/kernel/cpu/rdrand.c b/arch/x86/kernel/cpu/rdrand.c
index 8f216669ecb8..6f4b196fe97d 100644
--- a/arch/x86/kernel/cpu/rdrand.c
+++ b/arch/x86/kernel/cpu/rdrand.c
@@ -11,54 +11,51 @@
#include <asm/archrandom.h>
#include <asm/sections.h>
-static int __init x86_rdrand_setup(char *s)
-{
- setup_clear_cpu_cap(X86_FEATURE_RDRAND);
- setup_clear_cpu_cap(X86_FEATURE_RDSEED);
- return 1;
-}
-__setup("nordrand", x86_rdrand_setup);
-
/*
* RDRAND has Built-In-Self-Test (BIST) that runs on every invocation.
- * Run the instruction a few times as a sanity check.
- * If it fails, it is simple to disable RDRAND here.
+ * Run the instruction a few times as a sanity check. Also make sure
+ * it's not outputting the same value over and over, which has happened
+ * as a result of past CPU bugs.
+ *
+ * If it fails, it is simple to disable RDRAND and RDSEED here.
*/
-#define SANITY_CHECK_LOOPS 8
void x86_init_rdrand(struct cpuinfo_x86 *c)
{
- unsigned int changed = 0;
- unsigned long tmp, prev;
- int i;
-
- if (!cpu_has(c, X86_FEATURE_RDRAND))
- return;
-
- for (i = 0; i < SANITY_CHECK_LOOPS; i++) {
- if (!rdrand_long(&tmp)) {
- clear_cpu_cap(c, X86_FEATURE_RDRAND);
- pr_warn_once("rdrand: disabled\n");
- return;
+ enum { SAMPLES = 8, MIN_CHANGE = 5 };
+ unsigned long sample, prev;
+ bool failure = false;
+ size_t i, changed;
+
+ if (cpu_has(c, X86_FEATURE_RDRAND)) {
+ for (changed = 0, i = 0; i < SAMPLES; ++i) {
+ if (!rdrand_long(&sample)) {
+ failure = true;
+ break;
+ }
+ changed += i && sample != prev;
+ prev = sample;
}
+ if (changed < MIN_CHANGE)
+ failure = true;
}
- /*
- * Stupid sanity-check whether RDRAND does *actually* generate
- * some at least random-looking data.
- */
- prev = tmp;
- for (i = 0; i < SANITY_CHECK_LOOPS; i++) {
- if (rdrand_long(&tmp)) {
- if (prev != tmp)
- changed++;
-
- prev = tmp;
+ if (cpu_has(c, X86_FEATURE_RDSEED)) {
+ for (changed = 0, i = 0; i < SAMPLES; ++i) {
+ if (!rdseed_long(&sample)) {
+ failure = true;
+ break;
+ }
+ changed += i && sample != prev;
+ prev = sample;
}
+ if (changed < MIN_CHANGE)
+ failure = true;
}
- if (WARN_ON_ONCE(!changed))
- pr_emerg(
-"RDRAND gives funky smelling output, might consider not using it by booting with \"nordrand\"");
-
+ if (failure) {
+ clear_cpu_cap(c, X86_FEATURE_RDRAND);
+ clear_cpu_cap(c, X86_FEATURE_RDSEED);
+ pr_emerg("RDRAND and RDSEED are not reliable on this platform; disabling.\n");
+ }
}
--
2.35.1
On Sat, Jul 09, 2022 at 12:43:06PM +0200, Jason A. Donenfeld wrote:
> The decision of whether or not to trust RDRAND is controlled by the
> "random.trust_cpu" boot time parameter or the CONFIG_RANDOM_TRUST_CPU
> compile time default. The "nordrand" flag was added during the early
> days of RDRAND, when there were worries that merely using its values
> could compromise the RNG. However, these days, RDRAND values are not
> used directly but always go through the RNG's hash function, making
> "nordrand" no longer useful.
>
> Rather, the correct switch is "random.trust_cpu", which not only handles
> the relevant trust issue directly, but also is general to multiple CPU
> types, not just x86.
>
> However, x86 RDRAND does have a history of being occasionally
> problematic. Prior, when the kernel would notice something strange, it'd
> warn in dmesg and suggest enabling "nordrand". We can improve on that by
> making the test a little bit better and then taking the step of
> automatically disabling RDRAND if we detect it's problematic.
>
> Also extend the basic sanity test to RDSEED in addition to RDRAND, and
> disable both if either one fails.
>
> Cc: [email protected]
> Cc: Theodore Ts'o <[email protected]>
> Suggested-by: H. Peter Anvin <[email protected]>
> Suggested-by: Borislav Petkov <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>
> ---
> This is a tip-ish commit, but it relies on the CONFIG_ARCH_RANDOM commit
> in the random tree, so I'll take this through the random tree to avoid
> conflicts.
Acked-by: Borislav Petkov <[email protected]>
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
(HRB 36809, AG Nürnberg)
The decision of whether or not to trust RDRAND is controlled by the
"random.trust_cpu" boot time parameter or the CONFIG_RANDOM_TRUST_CPU
compile time default. The "nordrand" flag was added during the early
days of RDRAND, when there were worries that merely using its values
could compromise the RNG. However, these days, RDRAND values are not
used directly but always go through the RNG's hash function, making
"nordrand" no longer useful.
Rather, the correct switch is "random.trust_cpu", which not only handles
the relevant trust issue directly, but also is general to multiple CPU
types, not just x86.
However, x86 RDRAND does have a history of being occasionally
problematic. Prior, when the kernel would notice something strange, it'd
warn in dmesg and suggest enabling "nordrand". We can improve on that by
making the test a little bit better and then taking the step of
automatically disabling RDRAND if we detect it's problematic.
Also disable RDSEED if the RDRAND test fails.
Cc: [email protected]
Cc: Theodore Ts'o <[email protected]>
Suggested-by: H. Peter Anvin <[email protected]>
Suggested-by: Borislav Petkov <[email protected]>
Acked-by: Borislav Petkov <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
Changes v2->v3:
- We can't call rdseed in a loop like we can rdrand because it's meant
to fail (return false) more often.
.../admin-guide/kernel-parameters.txt | 5 --
arch/x86/kernel/cpu/amd.c | 2 +-
arch/x86/kernel/cpu/rdrand.c | 57 +++++++------------
3 files changed, 22 insertions(+), 42 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f2d26cb7e853..1e2307f11105 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3733,11 +3733,6 @@
noreplace-smp [X86-32,SMP] Don't replace SMP instructions
with UP alternatives
- nordrand [X86] Disable kernel use of the RDRAND and
- RDSEED instructions even if they are supported
- by the processor. RDRAND and RDSEED are still
- available to user space applications.
-
noresume [SWSUSP] Disables resume and restores original swap
space.
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 35d5288394cb..48276c0e479d 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -808,7 +808,7 @@ static void clear_rdrand_cpuid_bit(struct cpuinfo_x86 *c)
return;
/*
- * The nordrand option can clear X86_FEATURE_RDRAND, so check for
+ * The self-test can clear X86_FEATURE_RDRAND, so check for
* RDRAND support using the CPUID function directly.
*/
if (!(cpuid_ecx(1) & BIT(30)) || rdrand_force)
diff --git a/arch/x86/kernel/cpu/rdrand.c b/arch/x86/kernel/cpu/rdrand.c
index 8f216669ecb8..26a427fa84ea 100644
--- a/arch/x86/kernel/cpu/rdrand.c
+++ b/arch/x86/kernel/cpu/rdrand.c
@@ -11,54 +11,39 @@
#include <asm/archrandom.h>
#include <asm/sections.h>
-static int __init x86_rdrand_setup(char *s)
-{
- setup_clear_cpu_cap(X86_FEATURE_RDRAND);
- setup_clear_cpu_cap(X86_FEATURE_RDSEED);
- return 1;
-}
-__setup("nordrand", x86_rdrand_setup);
-
/*
* RDRAND has Built-In-Self-Test (BIST) that runs on every invocation.
- * Run the instruction a few times as a sanity check.
- * If it fails, it is simple to disable RDRAND here.
+ * Run the instruction a few times as a sanity check. Also make sure
+ * it's not outputting the same value over and over, which has happened
+ * as a result of past CPU bugs.
+ *
+ * If it fails, it is simple to disable RDRAND and RDSEED here.
*/
-#define SANITY_CHECK_LOOPS 8
void x86_init_rdrand(struct cpuinfo_x86 *c)
{
- unsigned int changed = 0;
- unsigned long tmp, prev;
- int i;
+ enum { SAMPLES = 8, MIN_CHANGE = 5 };
+ unsigned long sample, prev;
+ bool failure = false;
+ size_t i, changed;
if (!cpu_has(c, X86_FEATURE_RDRAND))
return;
- for (i = 0; i < SANITY_CHECK_LOOPS; i++) {
- if (!rdrand_long(&tmp)) {
- clear_cpu_cap(c, X86_FEATURE_RDRAND);
- pr_warn_once("rdrand: disabled\n");
- return;
+ for (changed = 0, i = 0; i < SAMPLES; ++i) {
+ if (!rdrand_long(&sample)) {
+ failure = true;
+ break;
}
+ changed += i && sample != prev;
+ prev = sample;
}
+ if (changed < MIN_CHANGE)
+ failure = true;
- /*
- * Stupid sanity-check whether RDRAND does *actually* generate
- * some at least random-looking data.
- */
- prev = tmp;
- for (i = 0; i < SANITY_CHECK_LOOPS; i++) {
- if (rdrand_long(&tmp)) {
- if (prev != tmp)
- changed++;
-
- prev = tmp;
- }
+ if (failure) {
+ clear_cpu_cap(c, X86_FEATURE_RDRAND);
+ clear_cpu_cap(c, X86_FEATURE_RDSEED);
+ pr_emerg("RDRAND is not reliable on this platform; disabling.\n");
}
-
- if (WARN_ON_ONCE(!changed))
- pr_emerg(
-"RDRAND gives funky smelling output, might consider not using it by booting with \"nordrand\"");
-
}
--
2.35.1