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

["Ard Biesheuvel" <ardb@kernel.org>]

On Mon, 14 Nov 2022 at 19:14, PierreGondois <pierre.gondois@arm.com> wrote:
>
> From: Pierre Gondois <pierre.gondois@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?


> Reported-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@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+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb+tianocore@kernel.org]
> ------------
>
>