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.falc...@gmail.com>; Michael D Kinney > <michael.d.kin...@intel.com>; Liming Gao <gaolim...@byosoft.com.cn>; > Zhiguang Liu <zhiguang....@intel.com>; Jason A . Donenfeld > <ja...@zx2c4.com> > 主题: [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 <ja...@zx2c4.com> > 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.falc...@gmail.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang....@intel.com> > Cc: Jason A. Donenfeld <ja...@zx2c4.com> > --- > 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 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97058): https://edk2.groups.io/g/devel/message/97058 Mute This Topic: https://groups.io/mt/95507944/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
