Reviewed-by: Jaben Carsey <jaben.car...@intel.com> Code change looks good visually.
> -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Heinrich Schuchardt > Sent: Thursday, May 09, 2019 8:24 PM > To: Gao, Zhichao <zhichao....@intel.com> > Cc: devel @ edk2 . groups . io <devel@edk2.groups.io>; Carsey, Jaben > <jaben.car...@intel.com>; Ni, Ray <ray...@intel.com>; Leif Lindholm > <leif.lindh...@linaro.org>; Gao, Liming <liming....@intel.com>; Heinrich > Schuchardt <xypron.g...@gmx.de> > Subject: [edk2-devel] [PATCH v3 1/1] ShellPkg/CommandLib: avoid NULL > derefence and memory leak > Importance: High > > Since TianoCore EDK2 commit d65f2cea36d1 ("ShellPkg/CommandLib: Locate > proper UnicodeCollation instance") in edk2 the UEFI Shell crashes if EFI > variable PlatformLang is not defined due to dereferencing gUnicodeCollation > gUnicodeCollation (= NULL) in ShellCommandRegisterCommandName(). > > Furthermore CommandInit() is leaking PlatformLang if > gUnicodeCollation != NULL. > > Close the memory leak and use the first UnicodeCollation instance if > PlatfomLang is not defined. > > Fixes: d65f2cea36d1 ("ShellPkg/CommandLib: Locate proper > UnicodeCollation > instance") > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de> > Reviewed-by: Zhichao Gao <zhichao....@intel.com> > --- > v3 > resend as quoted-printable > --- > .../UefiShellCommandLib/UefiShellCommandLib.c | 20 +++++++++++++----- > - > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > index ddc4bb1567..e60279e5ac 100644 > --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > +++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > @@ -80,12 +80,10 @@ CommandInit( > EFI_STATUS Status; > > CHAR8 *PlatformLang; > > > > - GetEfiGlobalVariable2 (EFI_PLATFORM_LANG_VARIABLE_NAME, > (VOID**)&PlatformLang, NULL); > > - if (PlatformLang == NULL) { > > - return EFI_UNSUPPORTED; > > - } > > - > > if (gUnicodeCollation == NULL) { > > + > > + GetEfiGlobalVariable2 (EFI_PLATFORM_LANG_VARIABLE_NAME, > (VOID**)&PlatformLang, NULL); > > + > > Status = gBS->LocateHandleBuffer ( > > ByProtocol, > > &gEfiUnicodeCollation2ProtocolGuid, > > @@ -113,6 +111,14 @@ CommandInit( > continue; > > } > > > > + // > > + // Without clue provided use the first Unicode Collation2 protocol. > > + // > > + if (PlatformLang == NULL) { > > + gUnicodeCollation = Uc; > > + break; > > + } > > + > > // > > // Find the best matching matching language from the supported > languages > > // of Unicode Collation2 protocol. > > @@ -132,7 +138,9 @@ CommandInit( > if (Handles != NULL) { > > FreePool (Handles); > > } > > - FreePool (PlatformLang); > > + if (PlatformLang != NULL) { > > + FreePool (PlatformLang); > > + } > > } > > > > return (gUnicodeCollation == NULL) ? EFI_UNSUPPORTED : EFI_SUCCESS; > > -- > 2.20.1 > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#40395): https://edk2.groups.io/g/devel/message/40395 > Mute This Topic: https://groups.io/mt/31573312/1760878 > Group Owner: devel+ow...@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [jaben.car...@intel.com] > -=-=-=-=-=-= -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40439): https://edk2.groups.io/g/devel/message/40439 Mute This Topic: https://groups.io/mt/31573312/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-