Re: [PATCH v1 0/2] Fixing RngDxe error for ARM/AARCH64

["PierreGondois" <pierre.gondois@arm.com>]

Hello Kun,

Thanks for the patch-set, there is another patch-set that also aims to fix the logic at:
https://edk2.groups.io/g/devel/message/104341

but I haven't got feedback so far. Would it be possible to try it out to see if
it also solves your issue ?

Regards,
Pierre

On 6/28/23 22:33, Kun Qin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4491
> 
> On an ARM system that does not support firmware TRNG, the current logic
> from RngDxe will cause the system to assert at the below line:
> `ASSERT (Index != mAvailableAlgoArrayCount);`
> 
> The reason seems to be:
> 1. When initializing the number of `mAvailableAlgoArrayCount`, the logic
> will only treat the zero guid of "PcdCpuRngSupportedAlgorithm" as a
> warning and still increment the counter because "RngGetBytes" might still
> succeed:
> https://github.com/tianocore/edk2/blob/1a39bdf2c53858ebb39e6de1362203c65c163c63/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c#L51C3-L51C3.
> 2. This will cause the main entry to publish the RNG protocol and accept
> further usage.
> 3. However, during usage, the zero guid is always filtered out:
> https://github.com/tianocore/edk2/blob/1a39bdf2c53858ebb39e6de1362203c65c163c63/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c#L91.
> Thus, this will cause the system to always not able to find the algorithm
> and fail the boot with an assert.
> 
> The suggestion is to at least make the logic of initializing
> "mAvailableAlgoArrayCount" consistent and filtering algorithm consistent.
> 
> In addition, the usage of `mAvailableAlgoArray` will always trigger a
> data abortion error, which is caused by buffer allocated is
> `RNG_AVAILABLE_ALGO_MAX` number of bytes, which should be
> `RNG_AVAILABLE_ALGO_MAX` nubmer of EFI_RNG_ALGORITHM.
> 
> This patch fixed the 2 issues above. The change is verified on QEMU
> virtual platform and proprietary physical platform.
> 
> Patch v1 branch: https://github.com/kuqin12/edk2/tree/fix_rng_edk2_v1
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> 
> Kun Qin (2):
>    SecurityPkg: RngDxe: Unify handling of zero guid
>    SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator
> 
>   SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c | 9 +++++----
>   SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c         | 2 +-
>   2 files changed, 6 insertions(+), 5 deletions(-)
>