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

["Kun Qin" <kuqin12@gmail.com>]

Hi Sami,

Your suggestion in https://edk2.groups.io/g/devel/message/106511 works 
properly during my test.

But I think we still need to keep the `+      &RngGuid, ` change below 
as a bug fix?

Thanks,
Kun

On 6/29/2023 3:28 AM, Sami Mujawar wrote:
> 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;
>>   }
>
>
> 
>
>