Pedro: To keep the same behavior, I suggest to only update BaseRngLibConstructor() API with TestRdRand, don't touch other APIs. Thanks Liming -----邮件原件----- 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Pedro Falcato 发送时间: 2022年11月23日 6:31 收件人: devel@edk2.groups.io 抄送: Pedro Falcato <pedro.falcato@...>; Michael D Kinney <michael.d.kinney@...>; Liming Gao <gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>; Jason A . Donenfeld <Jason@...> 主题: [edk2-devel] [PATCH v4 1/1] MdePkg/BaseRngLib: Add a smoketest for RDRAND and check CPUID
RDRAND has notoriously been broken many times over its lifespan. Add a smoketest to RDRAND, in order to better sniff out potential security concerns.
Also add a proper CPUID test in order to support older CPUs which may not have it; it was previously being tested but then promptly ignored.
Testing algorithm inspired by linux's arch/x86/kernel/cpu/rdrand.c :x86_init_rdrand() per commit 049f9ae9..
Many thanks to Jason Donenfeld for relicensing his linux RDRAND detection code to MIT and the public domain.
On Tue, Nov 22, 2022 at 2:21 PM Jason A. Donenfeld <Jason@...> wrote: <..>
I (re)wrote that function in Linux. I hereby relicense it as MIT, and also place it into public domain. Do with it what you will now.
Jason BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4163
Signed-off-by: Pedro Falcato <pedro.falcato@...> Cc: Michael D Kinney <michael.d.kinney@...> Cc: Liming Gao <gaoliming@...> Cc: Zhiguang Liu <zhiguang.liu@...> Cc: Jason A. Donenfeld <Jason@...> --- v4: Added a doxygen comment to the TestRdRand() function v3: Addressed mailing list comments v2: Replaced the original v1 naive detection with a better, although still not great, detection. MdePkg/Library/BaseRngLib/Rand/RdRand.c | 99 +++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 8 deletions(-)
diff --git a/MdePkg/Library/BaseRngLib/Rand/RdRand.c b/MdePkg/Library/BaseRngLib/Rand/RdRand.c index 070d41e2555f..ff99436dbbbd 100644 --- a/MdePkg/Library/BaseRngLib/Rand/RdRand.c +++ b/MdePkg/Library/BaseRngLib/Rand/RdRand.c @@ -2,6 +2,7 @@ Random number generator services that uses RdRand instruction access to provide high-quality random numbers.
+Copyright (c) 2022, Pedro Falcato. All rights reserved.<BR> Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
@@ -22,6 +23,88 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
STATIC BOOLEAN mRdRandSupported;
+// +// Intel SDM says 10 tries is good enough for reliable RDRAND usage. +// +#define RDRAND_RETRIES 10 + +#define RDRAND_TEST_SAMPLES 8 + +#define RDRAND_MIN_CHANGE 5 + +// +// Add a define for native-word RDRAND, just for the test. +// +#ifdef MDE_CPU_X64 +#define AsmRdRand AsmRdRand64 +#else +#define AsmRdRand AsmRdRand32 +#endif + +/** + Tests RDRAND for broken implementations. + + @retval TRUE RDRAND is reliable (and hopefully safe). + @retval FALSE RDRAND is unreliable and should be disabled, despite CPUID. + +**/ +STATIC +BOOLEAN +TestRdRand ( + VOID + ) +{ + // + // Test for notoriously broken rdrand implementations that always return the same + // value, like the Zen 3 uarch (all-1s) or other several AMD families on suspend/resume (also all-1s). + // Note that this should be expanded to extensively test for other sorts of possible errata. + // + + // + // Our algorithm samples rdrand $RDRAND_TEST_SAMPLES times and expects + // a different result $RDRAND_MIN_CHANGE times for reliable RDRAND usage. + // + UINTN Prev; + UINT8 Idx; + UINT8 TestIteration; + UINT32 Changed; + + Changed = 0; + + for (TestIteration = 0; TestIteration < RDRAND_TEST_SAMPLES; TestIteration++) { + UINTN Sample; + // + // Note: We use a retry loop for rdrand. Normal users get this in BaseRng.c + // Any failure to get a random number will assume RDRAND does not work. + // + for (Idx = 0; Idx < RDRAND_RETRIES; Idx++) { + if (AsmRdRand (&Sample)) { + break; + } + } + + if (Idx == RDRAND_RETRIES) { + DEBUG ((DEBUG_ERROR, "BaseRngLib/x86: CPU BUG: Failed to get an RDRAND random number - disabling\n")); + return FALSE; + } + + if (TestIteration != 0) { + Changed += Sample != Prev; + } + + Prev = Sample; + } + + if (Changed < RDRAND_MIN_CHANGE) { + DEBUG ((DEBUG_ERROR, "BaseRngLib/x86: CPU BUG: RDRAND not reliable - disabling\n")); + return FALSE; + } + + return TRUE; +} + +#undef AsmRdRand + /** The constructor function checks whether or not RDRAND instruction is supported by the host hardware. @@ -46,10 +129,13 @@ BaseRngLibConstructor ( // CPUID. A value of 1 indicates that processor support RDRAND instruction. // AsmCpuid (1, 0, 0, &RegEcx, 0); - ASSERT ((RegEcx & RDRAND_MASK) == RDRAND_MASK);
mRdRandSupported = ((RegEcx & RDRAND_MASK) == RDRAND_MASK);
+ if (mRdRandSupported) { + mRdRandSupported = TestRdRand (); + } + return EFI_SUCCESS; }
@@ -68,6 +154,7 @@ ArchGetRandomNumber16 ( OUT UINT16 *Rand ) { + ASSERT (mRdRandSupported); return AsmRdRand16 (Rand); }
@@ -86,6 +173,7 @@ ArchGetRandomNumber32 ( OUT UINT32 *Rand ) { + ASSERT (mRdRandSupported); return AsmRdRand32 (Rand); }
@@ -104,6 +192,7 @@ ArchGetRandomNumber64 ( OUT UINT64 *Rand ) { + ASSERT (mRdRandSupported); return AsmRdRand64 (Rand); }
@@ -120,11 +209,5 @@ ArchIsRngSupported ( VOID ) { - /* - Existing software depends on this always returning TRUE, so for - now hard-code it. - - return mRdRandSupported; - */ - return TRUE; + return mRdRandSupported; } -- 2.38.1
|