From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.5631.1682509174785685560 for ; Wed, 26 Apr 2023 04:39:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=efrbBamR; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1682509173; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=aprUVYWQvQmbztrQa8XxmXW4lPzqfUw4yLT68zdHRmo=; b=efrbBamR9o5mKlAMiTCK610BqB7bvjxYC0za0D+KWMzt9JCqwJLc2zja4/2DRJlKK8cMH8 T8ZuaViVZ4RaBSkRK811BjjjXRm6POeSW7bN5/FHPi/44pbbT6JWxaLTVUxUN4xtRIVmX5 ynrUBTeweOsOj49nXY0Fahgo9AtlFGs= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-594-hdTYfMduPE-iYzHH2Z3-8w-1; Wed, 26 Apr 2023 07:39:30 -0400 X-MC-Unique: hdTYfMduPE-iYzHH2Z3-8w-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 89B258C446F; Wed, 26 Apr 2023 11:39:30 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.194.1]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4DE1040C6E67; Wed, 26 Apr 2023 11:39:30 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 9573F1800632; Wed, 26 Apr 2023 13:39:27 +0200 (CEST) From: "Gerd Hoffmann" To: devel@edk2.groups.io Cc: Ray Ni , Pawel Polawski , Oliver Steffen , Zhichao Gao , Gerd Hoffmann Subject: [PATCH 1/1] ShellPkgDisconnect: zero-initialize handles Date: Wed, 26 Apr 2023 13:39:27 +0200 Message-Id: <20230426113927.287438-1-kraxel@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true 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 --- .../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