Re: [PATCH v1 7/8] SecurityPkg/RngDxe: Select safe default Rng algorithm

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Pierre,

I think this patch would not be required if my suggestions for patch 6/8 
are adopted.

Regards,

Sami Mujawar

On 09/05/2023 08:40 am, pierre.gondois@arm.com wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> The first element of mAvailableAlgoArray should be the default
> algorithm to avoid going through a selection process at each
> RngGetRNG() call.
> Once all the available Rng algorithms have been probed, place
> a safe Rng algorithm at the first position of mAvailableAlgoArray.
>
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>   .../RngDxe/AArch64/AArch64Algo.c              | 48 ++++++++++++++++++-
>   1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
> index a1ff7bd58fda..ed236b2e8141 100644
> --- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
> +++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
> @@ -17,6 +17,50 @@
>   // Maximum number of Rng algorithms.
>   #define RNG_AVAILABLE_ALGO_MAX  2
>   
> +/** mAvailableAlgoArray[0] should contain the default Rng algorithm.
> +    The Rng algorithm at the first index might be unsafe.
> +    If a safe algorithm is available, choose it as the default one.
> +**/
> +VOID
> +EFIAPI
> +RngFindDefaultAlgo (
> +  VOID
> +  )
> +{
> +  EFI_RNG_ALGORITHM  *CurAlgo;
> +  EFI_RNG_ALGORITHM  TmpGuid;
> +  UINTN              Index;
> +
> +  CurAlgo = &mAvailableAlgoArray[0];
> +
> +  if (IsZeroGuid (CurAlgo) ||
> +      !CompareGuid (CurAlgo, &gEfiRngAlgorithmUnSafe))
> +  {
> +    // mAvailableAlgoArray[0] is a valid Rng algorithm.
> +    return;
> +  }
> +
> +  for (Index = 1; Index < mAvailableAlgoArrayCount; Index++) {
> +    CurAlgo = &mAvailableAlgoArray[Index];
> +    if (!IsZeroGuid (CurAlgo) ||
> +        CompareGuid (CurAlgo, &gEfiRngAlgorithmUnSafe))
> +    {
> +      break;
> +    }
> +  }
> +
> +  if (Index == mAvailableAlgoArrayCount) {
> +    // No valid Rng algorithm available.
> +    return;
> +  }
> +
> +  CopyMem (&TmpGuid, CurAlgo, sizeof (EFI_RNG_ALGORITHM));
> +  CopyMem (CurAlgo, &mAvailableAlgoArray[0], sizeof (EFI_RNG_ALGORITHM));
> +  CopyMem (&mAvailableAlgoArray[0], &TmpGuid, sizeof (EFI_RNG_ALGORITHM));
> +
> +  return;
> +}
> +
>   /** Allocate and initialize mAvailableAlgoArray with the available
>       Rng algorithms. Also update mAvailableAlgoArrayCount.
>   
> @@ -45,7 +89,7 @@ GetAvailableAlgorithms (
>     if (!EFI_ERROR (Status)) {
>       CopyMem (
>         &mAvailableAlgoArray[mAvailableAlgoArrayCount],
> -      RngGuid,
> +      &RngGuid,
>         sizeof (RngGuid)
>         );
>       mAvailableAlgoArrayCount++;
> @@ -68,5 +112,7 @@ GetAvailableAlgorithms (
>       mAvailableAlgoArrayCount++;
>     }
>   
> +  RngFindDefaultAlgo ();
> +
>     return EFI_SUCCESS;
>   }