[edk2-devel] [PATCH v2 1/1] SecurityPkg/RngDxe: Fix Rng algo selection for Arm

[Ard Biesheuvel]

On Wed, 16 Nov 2022 at 16:02, PierreGondois <pierre.gondois at arm.com> wrote:
>
> From: Pierre Gondois <pierre.gondois at arm.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151
>
> The EFI_RNG_PROTOCOL can advertise multiple algorithms through
> Guids. The PcdCpuRngSupportedAlgorithm contains a Guid that
> can be configured. It represents the algorithm used in RngLib.
> PcdCpuRngSupportedAlgorithm is set to the Zero Guid for KvmTool.
>
> When running KvmTool on a platform platform only having the RngLib,
> the only Guid available for EFI_RNG_PROTOCOL will be the zero Guid.
>
> To select the default algorithm in EFI_RNG_PROTOCOL.GetRng():
> a. Zero Guids are skipped
> b. If no algorithm is found, an ASSERT is triggered
>
> To allow using the RngLib to be used for the case above, Zero Guids
> should not be skipped (a.).
> If no algorithm is found, don't prevent from booting on DEBUG builds
> (b.).
>
> Allow Zero Guids to be selected and don't ASSERT if no 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.
>
> Reported-by: Sami Mujawar <sami.mujawar at arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois at arm.com>

I am still confused by this.

Does this mean we might register the RNG protocol if we don't have
anything to back it up?

> ---
>
> Notes:
>     v2:
>     - Reformulate commit message.
>     - Do not warn if no algorithm is found as the message
>       would be printed on non-Arm platforms.
>
>  .../RandomNumberGenerator/RngDxe/ArmRngDxe.c      | 15 +++------------
>  1 file changed, 3 insertions(+), 12 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;
> --
> 2.25.1
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#96434): https://edk2.groups.io/g/devel/message/96434
> Mute This Topic: https://groups.io/mt/95067856/5717338
> Group Owner: devel+owner at edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb+tianocore at kernel.org]
> ------------
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96491): https://edk2.groups.io/g/devel/message/96491
Mute This Topic: https://groups.io/mt/95067856/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-