From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from foss.arm.com (foss.arm.com [217.140.110.172])
 by mx.groups.io with SMTP id smtpd.web11.2913.1688021031921224678
 for <devel@edk2.groups.io>;
 Wed, 28 Jun 2023 23:43:52 -0700
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com)
Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14])
	by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CE52BC14;
	Wed, 28 Jun 2023 23:44:34 -0700 (PDT)
Received: from [192.168.1.12] (unknown [172.31.20.19])
	by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 23E7D3F73F;
	Wed, 28 Jun 2023 23:43:49 -0700 (PDT)
Message-ID: <2c8316c4-2e12-87c2-a90b-16bcdb9e2749@arm.com>
Date: Thu, 29 Jun 2023 08:43:43 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v1 0/2] Fixing RngDxe error for ARM/AARCH64
To: Kun Qin <kuqin12@gmail.com>, devel@edk2.groups.io
Cc: Jiewen Yao <jiewen.yao@intel.com>, Jian J Wang <jian.j.wang@intel.com>,
 Sami Mujawar <Sami.Mujawar@arm.com>
References: <20230628203357.2001-1-kuqin12@gmail.com>
From: "PierreGondois" <pierre.gondois@arm.com>
In-Reply-To: <20230628203357.2001-1-kuqin12@gmail.com>
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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(-)
>