On 11/14/22 20:12, Ard Biesheuvel wrote:
On Mon, 14 Nov 2022 at 19:14, PierreGondois <pierre.gond...@arm.com> wrote:
From: Pierre Gondois <pierre.gond...@arm.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151
PcdCpuRngSupportedAlgorithm is set to the Zero Guid for KvmTool
since it is not possible to predict which algorithm will be
implemented for Arm's FEAT_RNG in the host. Current handling is:
- skipping the Zero Guid, which should not happen to handle
KvmTool's case,
- triggering an ASSERT if no algorithm was found. However having
no Rng algorithm is a valid case,
Correctly handle the Zero Guid case and replace the ASSERT by a
warning message when no Rng algorithm is found.
Also simplify the selection of the Rng algorithm when the default
one is selected by just picking up the first element of
mAvailableAlgoArray.
Can you explain a bit more please
What is supposed to happen?
What happens instead?
Why is this patch the correct way to address this issue?
Hello Ard,
mAvailableAlgoArray in RngDxe stores the availble Rng algorithms.
The array is populated in GetAvailableAlgorithms() by testing the
possible algorithms. This is done when the driver is loaded.
PcdCpuRngSupportedAlgorithm is the Guid used to identify the algorithm
behind the RngLib, i.e. the RNDR instruction. It is set to a zero Guid
for KvmTool since it isn't possible to set a unique value for all the
hosts.
When using RngDxe, it is possible to either specify which Rng algorithm
to use or to use a default one. When requesting the default one, the
first element of mAvailableAlgoArray should be used. This is an
arbitrary decision, but mAvailableAlgoArray will always be populated
in the same the order.
On arm64, the 2 possible algorithms are, in order:
- the RngLib, which should use the RNDR instruction
- the TRNG
If there is no TRNG, the only Guid available will be the one for RNDR,
i.e. PcdCpuRngSupportedAlgorithm (zero Guid).
The current code:
- skips the zero Guid, which should not be the case since it represents
a valid algorithm,
- ASSERT if no algorithm is found. If no algorithm is found, we
should not stop, just return an error code, and the caller will act
on this error. A warning is however added in case there is no
algorithm found,
Regards,
Pierre
Reported-by: Sami Mujawar <sami.muja...@arm.com>
Signed-off-by: Pierre Gondois <pierre.gond...@arm.com>
---
.../RandomNumberGenerator/RngDxe/ArmRngDxe.c | 15 +++------------
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c | 8 +++++++-
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
index 5ba319899ce9..722d53386373 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
@@ -76,7 +76,6 @@ RngGetRNG (
)
{
EFI_STATUS Status;
- UINTN Index;
if ((This == NULL) || (RNGValueLength == 0) || (RNGValue == NULL)) {
return EFI_INVALID_PARAMETER;
@@ -86,21 +85,13 @@ RngGetRNG (
//
// Use the default RNG algorithm if RNGAlgorithm is NULL.
//
- for (Index = 0; Index < mAvailableAlgoArrayCount; Index++) {
- if (!IsZeroGuid (&mAvailableAlgoArray[Index])) {
- RNGAlgorithm = &mAvailableAlgoArray[Index];
- goto FoundAlgo;
- }
- }
-
- if (Index == mAvailableAlgoArrayCount) {
- // No algorithm available.
- ASSERT (Index != mAvailableAlgoArrayCount);
+ if (mAvailableAlgoArrayCount != 0) {
+ RNGAlgorithm = &mAvailableAlgoArray[0];
+ } else {
return EFI_DEVICE_ERROR;
}
}
-FoundAlgo:
if (CompareGuid (RNGAlgorithm, PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
Status = RngGetBytes (RNGValueLength, RNGValue);
return Status;
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
index 421abb52b8bf..403b31b73609 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
@@ -21,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/RngLib.h>
#include <Protocol/Rng.h>
@@ -80,7 +81,12 @@ RngDriverEntry (
//
// Get the list of available algorithm.
//
- return GetAvailableAlgorithms ();
+ Status = GetAvailableAlgorithms ();
+ if (mAvailableAlgoArrayCount == 0) {
+ DEBUG ((DEBUG_WARN, "No Rng algorithm found in RngDxe.\n"));
+ }
+
+ return Status;
}
/**
--
2.25.1
------------
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96356): https://edk2.groups.io/g/devel/message/96356
Mute This Topic: https://groups.io/mt/95025606/5717338
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb+tianoc...@kernel.org]
------------
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96389): https://edk2.groups.io/g/devel/message/96389
Mute This Topic: https://groups.io/mt/95025606/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
