From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from walk.intel-email.com (walk.intel-email.com [101.227.64.242]) by mx.groups.io with SMTP id smtpd.web10.4632.1670378248223277874 for ; Tue, 06 Dec 2022 17:57:29 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@byosoft.com.cn header.s=cloud-union header.b=era6cnm6; spf=pass (domain: byosoft.com.cn, ip: 101.227.64.242, mailfrom: gaoliming@byosoft.com.cn) Received: from walk.intel-email.com (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id EB1E5CD1F6CF for ; Wed, 7 Dec 2022 09:57:25 +0800 (CST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=byosoft.com.cn; s=cloud-union; t=1670378246; bh=iiB+W459YttVO9+dT/nwIJy49DE6G+S042W7EPFedZY=; h=From:To:Cc:References:In-Reply-To:Subject:Date; b=era6cnm66b9hTWodVaY/WBDPLhrNFbj01Aq6Hsf9fJHk0SrtSnyAq/ydkGDbTYb85 qYp02NkHLfEyqZD0+uyE08jQS17TrlMaIJTO4TMDUyRawN6zjQSiN/9eiMpyDeJjVb sQlUDAuGEGlgzEgPxwV9BPCF6I8pHI7Qjw4s/Lw4= Received: from localhost (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id E6FB6CD1F6CA for ; Wed, 7 Dec 2022 09:57:25 +0800 (CST) Received: from walk.intel-email.com (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id B6F6ACD1F689 for ; Wed, 7 Dec 2022 09:57:25 +0800 (CST) Authentication-Results: walk.intel-email.com; none Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by walk.intel-email.com (Postfix) with SMTP id 4278CCD1F762 for ; Wed, 7 Dec 2022 09:57:22 +0800 (CST) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Wed, 07 Dec 2022 09:57:20 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , Cc: "'Michael D Kinney'" , "'Zhiguang Liu'" , "'Jason A . Donenfeld'" References: <20221122223103.597984-1-pedro.falcato@gmail.com> In-Reply-To: <20221122223103.597984-1-pedro.falcato@gmail.com> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHY0IDEvMV0gTWRlUGtnL0Jhc2VSbmdMaWI6IEFkZCBhIHNtb2tldGVzdCBmb3IgUkRSQU5EIGFuZCBjaGVjayBDUFVJRA==?= Date: Wed, 7 Dec 2022 09:57:21 +0800 Message-ID: <001001d909df$3ed34350$bc79c9f0$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQIqorcjQ5MuFYAAr5bQ83x6PhX8vK2+eJUQ Sender: "gaoliming" Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Pedro: To keep the same behavior, I suggest to only update BaseRngLibConstructor() API with TestRdRand, don't touch other APIs. Thanks Liming > -----=D3=CA=BC=FE=D4=AD=BC=FE----- > =B7=A2=BC=FE=C8=CB: devel@edk2.groups.io =B4=FA=B1= =ED Pedro Falcato > =B7=A2=CB=CD=CA=B1=BC=E4: 2022=C4=EA11=D4=C223=C8=D5 6:31 > =CA=D5=BC=FE=C8=CB: devel@edk2.groups.io > =B3=AD=CB=CD: Pedro Falcato ; Michael D Kinney > ; Liming Gao ; > Zhiguang Liu ; Jason A . Donenfeld > > =D6=F7=CC=E2: [edk2-devel] [PATCH v4 1/1] MdePkg/BaseRngLib: Add a smoket= est for > RDRAND and check CPUID >=20 > RDRAND has notoriously been broken many times over its lifespan. > Add a smoketest to RDRAND, in order to better sniff out potential > security concerns. >=20 > 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. >=20 > Testing algorithm inspired by linux's arch/x86/kernel/cpu/rdrand.c > :x86_init_rdrand() per commit 049f9ae9.. >=20 > Many thanks to Jason Donenfeld for relicensing his linux RDRAND detection > code to MIT and the public domain. >=20 > >On Tue, Nov 22, 2022 at 2:21 PM Jason A. Donenfeld > wrote: > <..> > > I (re)wrote that function in Linux. I hereby relicense it as MIT, an= d > > also place it into public domain. Do with it what you will now. > > > > Jason >=20 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4163 >=20 > Signed-off-by: Pedro Falcato > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Zhiguang Liu > Cc: Jason A. Donenfeld > --- > 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 stil= l not > great, detection. > MdePkg/Library/BaseRngLib/Rand/RdRand.c | 99 > +++++++++++++++++++++++-- > 1 file changed, 91 insertions(+), 8 deletions(-) >=20 > 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. >=20 > +Copyright (c) 2022, Pedro Falcato. All rights reserved.
> Copyright (c) 2021, NUVIA Inc. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.
>=20 > @@ -22,6 +23,88 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > STATIC BOOLEAN mRdRandSupported; >=20 > +// > +// 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 =3D 0; > + > + for (TestIteration =3D 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 =3D 0; Idx < RDRAND_RETRIES; Idx++) { > + if (AsmRdRand (&Sample)) { > + break; > + } > + } > + > + if (Idx =3D=3D RDRAND_RETRIES) { > + DEBUG ((DEBUG_ERROR, "BaseRngLib/x86: CPU BUG: Failed to get > an RDRAND random number - disabling\n")); > + return FALSE; > + } > + > + if (TestIteration !=3D 0) { > + Changed +=3D Sample !=3D Prev; > + } > + > + Prev =3D 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) =3D=3D RDRAND_MASK); >=20 > mRdRandSupported =3D ((RegEcx & RDRAND_MASK) =3D=3D RDRAND_MASK); >=20 > + if (mRdRandSupported) { > + mRdRandSupported =3D TestRdRand (); > + } > + > return EFI_SUCCESS; > } >=20 > @@ -68,6 +154,7 @@ ArchGetRandomNumber16 ( > OUT UINT16 *Rand > ) > { > + ASSERT (mRdRandSupported); > return AsmRdRand16 (Rand); > } >=20 > @@ -86,6 +173,7 @@ ArchGetRandomNumber32 ( > OUT UINT32 *Rand > ) > { > + ASSERT (mRdRandSupported); > return AsmRdRand32 (Rand); > } >=20 > @@ -104,6 +192,7 @@ ArchGetRandomNumber64 ( > OUT UINT64 *Rand > ) > { > + ASSERT (mRdRandSupported); > return AsmRdRand64 (Rand); > } >=20 > @@ -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 >=20 >=20 >=20 >=20 >=20