Ping.
Any comments on this?
On Wed, Apr 26, 2023 at 01:39:27PM +0200, Gerd Hoffmann wrote:
> In case ShellConvertStringToUint64() fails the Handles are left
> uninitialized. That can for example happen for Handle2 and Handle3 in
> case only one parameter was specified on the command line. Which can
> trigger the ASSERT() in line 185.
>
> Reproducer: boot ovmf to efi shell in qemu, using q35 machine type, then
> try disconnect the sata controller in efi shell.
>
> Fix that by explicitly setting them to NULL in that case. While being
> at it also simplify the logic and avoid pointlessly calling
> ShellConvertStringToUint64() in case ParamN is NULL.
>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> .../UefiShellDriver1CommandsLib/Disconnect.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
> b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
> index fd49d1f7ceb4..fac6463e3c28 100644
> --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
> +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c
> @@ -160,16 +160,23 @@ ShellCommandRunDisconnect (
> Param1 = ShellCommandLineGetRawValue (Package, 1);
> Param2 = ShellCommandLineGetRawValue (Package, 2);
> Param3 = ShellCommandLineGetRawValue (Package, 3);
> - if (!EFI_ERROR (ShellConvertStringToUint64 (Param1, &Intermediate1,
> TRUE, FALSE))) {
> - Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle
> ((UINTN)Intermediate1) : NULL;
> +
> + if (Param1 && !EFI_ERROR (ShellConvertStringToUint64 (Param1,
> &Intermediate1, TRUE, FALSE))) {
> + Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate1);
> + } else {
> + Handle1 = NULL;
> }
>
> - if (!EFI_ERROR (ShellConvertStringToUint64 (Param2, &Intermediate2,
> TRUE, FALSE))) {
> - Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle
> ((UINTN)Intermediate2) : NULL;
> + if (Param2 && !EFI_ERROR (ShellConvertStringToUint64 (Param2,
> &Intermediate2, TRUE, FALSE))) {
> + Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate2);
> + } else {
> + Handle2 = NULL;
> }
>
> - if (!EFI_ERROR (ShellConvertStringToUint64 (Param3, &Intermediate3,
> TRUE, FALSE))) {
> - Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle
> ((UINTN)Intermediate3) : NULL;
> + if (Param3 && !EFI_ERROR (ShellConvertStringToUint64 (Param3,
> &Intermediate3, TRUE, FALSE))) {
> + Handle3 = ConvertHandleIndexToHandle ((UINTN)Intermediate3);
> + } else {
> + Handle3 = NULL;
> }
>
> if ((Param1 != NULL) && (Handle1 == NULL)) {
> --
> 2.40.0
>
--
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103982): https://edk2.groups.io/g/devel/message/103982
Mute This Topic: https://groups.io/mt/98512679/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
