[edk2-devel] [PATCH 1/1] MdePkg/BaseRngLib: Add a smoketest for RDRAND and check CPUID

[Pedro Falcato]

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.

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>
---
 MdePkg/Library/BaseRngLib/Rand/RdRand.c | 81 ++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 8 deletions(-)

diff --git a/MdePkg/Library/BaseRngLib/Rand/RdRand.c 
b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
index 070d41e2555f..ce9768955359 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,70 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 STATIC BOOLEAN  mRdRandSupported;
 
+//
+// Intel SDM says 10 tries is good enough for reliable RDRAND usage.
+// We'll double this to 20 just to be safe, since a failure when testing
+// makes RDRAND unavailable.
+//
+#define RDRAND_RETRIES  20
+
+#define RDRAND_TEST_TRIES  10
+
+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. This testing is quite naive.
+  //
+  UINT32   RandomNum;
+  BOOLEAN  HasRandomNum;
+  UINT8    Idx;
+  UINT8    TestIteration;
+
+  HasRandomNum = FALSE;
+
+  for (TestIteration = 0; TestIteration < RDRAND_TEST_TRIES; TestIteration++) {
+    UINT32  Tmp;
+    //
+    // 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 (AsmRdRand32 (&Tmp)) {
+        break;
+      }
+    }
+
+    if (Idx == RDRAND_RETRIES) {
+      DEBUG ((DEBUG_ERROR, "BaseRngLib/x86: CPU BUG: Failed to get an RDRAND 
random number - disabling\n"));
+      return FALSE;
+    }
+
+    if (HasRandomNum) {
+      if (RandomNum != Tmp) {
+        //
+        // We got a different number, so take it the RNG works.
+        //
+        DEBUG ((DEBUG_INFO, "BaseRngLib/x86: RDRAND test complete.\n"));
+        return TRUE;
+      }
+    }
+
+    RandomNum    = Tmp;
+    HasRandomNum = TRUE;
+  }
+
+  DEBUG ((DEBUG_ERROR, "BaseRngLib/x86: CPU BUG: RDRAND always returns the 
same result %x - disabling\n", RandomNum));
+
+  return FALSE;
+}
+
 /**
   The constructor function checks whether or not RDRAND instruction is 
supported
   by the host hardware.
@@ -46,10 +111,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 +136,7 @@ ArchGetRandomNumber16 (
   OUT     UINT16  *Rand
   )
 {
+  ASSERT (mRdRandSupported);
   return AsmRdRand16 (Rand);
 }
 
@@ -86,6 +155,7 @@ ArchGetRandomNumber32 (
   OUT     UINT32  *Rand
   )
 {
+  ASSERT (mRdRandSupported);
   return AsmRdRand32 (Rand);
 }
 
@@ -104,6 +174,7 @@ ArchGetRandomNumber64 (
   OUT     UINT64  *Rand
   )
 {
+  ASSERT (mRdRandSupported);
   return AsmRdRand64 (Rand);
 }
 
@@ -120,11 +191,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 (#96552): https://edk2.groups.io/g/devel/message/96552
Mute This Topic: https://groups.io/mt/95195631/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-