Hi Pierre/Sami,
Thanks for the information. I tried the patch set you referred, it seems
to have resolved
the zero GUID handling issue! So I am okay to drop the commit here:
https://edk2.groups.io/g/devel/message/106484
I also have a few questions on the patch set beyond functionality, we
can discuss it there.
However, this specific patch is still needed to fix the access violation
issue (thanks for the
reviewed-by tag, Sami):
https://edk2.groups.io/g/devel/message/106485
I can send a v2 to reflect the feedback above.
Thanks,
Kun
On 6/28/2023 11:43 PM, Pierre Gondois wrote:
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....@intel.com>
Cc: Jian J Wang <jian.j.w...@intel.com>
Cc: Sami Mujawar <sami.muja...@arm.com>
Cc: Pierre Gondois <pierre.gond...@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(-)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106528): https://edk2.groups.io/g/devel/message/106528
Mute This Topic: https://groups.io/mt/99839045/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
