Reviewed-by: Zhichao Gao <zhichao....@intel.com>
Thanks,
Zhichao
> -----Original Message-----
> From: Gerd Hoffmann <kra...@redhat.com>
> Sent: Thursday, May 4, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray...@intel.com>; Pawel Polawski <ppola...@redhat.com>;
> Oliver Steffen <ostef...@redhat.com>; Gao, Zhichao
> <zhichao....@intel.com>
> Subject: Re: [PATCH 1/1] ShellPkgDisconnect: zero-initialize handles
>
> 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 (#104450): https://edk2.groups.io/g/devel/message/104450
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]
-=-=-=-=-=-=-=-=-=-=-=-
