public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kun Qin" <kuqin12@gmail.com>
To: 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>,
	Pierre Gondois <pierre.gondois@arm.com>
Subject: [PATCH v1 0/2] Fixing RngDxe error for ARM/AARCH64
Date: Wed, 28 Jun 2023 13:33:54 -0700	[thread overview]
Message-ID: <20230628203357.2001-1-kuqin12@gmail.com> (raw)

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

-- 
2.41.0.windows.1


             reply	other threads:[~2023-06-28 20:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-28 20:33 Kun Qin [this message]
2023-06-28 20:33 ` [PATCH v1 1/2] SecurityPkg: RngDxe: Unify handling of zero guid Kun Qin
2023-06-29 10:33   ` Sami Mujawar
2023-06-28 20:33 ` [PATCH v1 2/2] SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator Kun Qin
2023-06-29 10:33   ` Sami Mujawar
2023-06-29  6:43 ` [PATCH v1 0/2] Fixing RngDxe error for ARM/AARCH64 PierreGondois
2023-06-29 20:28   ` Kun Qin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230628203357.2001-1-kuqin12@gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox