From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.136, mailfrom: jaben.carsey@intel.com) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by groups.io with SMTP; Fri, 10 May 2019 07:32:19 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 May 2019 07:32:18 -0700 X-ExtLoop1: 1 Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga005.fm.intel.com with ESMTP; 10 May 2019 07:32:18 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 10 May 2019 07:32:18 -0700 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.107]) by fmsmsx115.amr.corp.intel.com ([169.254.4.68]) with mapi id 14.03.0415.000; Fri, 10 May 2019 07:32:18 -0700 From: "Carsey, Jaben" To: "devel@edk2.groups.io" , "xypron.glpk@gmx.de" , "Gao, Zhichao" CC: "Ni, Ray" , Leif Lindholm , "Gao, Liming" Subject: Re: [edk2-devel] [PATCH v3 1/1] ShellPkg/CommandLib: avoid NULL derefence and memory leak Thread-Topic: [edk2-devel] [PATCH v3 1/1] ShellPkg/CommandLib: avoid NULL derefence and memory leak Thread-Index: AQHVBt/qaJfzJhTOgEK2j2Y7SQ95iqZka76w Date: Fri, 10 May 2019 14:32:17 +0000 Message-ID: References: <20190510032419.14540-1-xypron.glpk@gmx.de> In-Reply-To: <20190510032419.14540-1-xypron.glpk@gmx.de> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTc1MjM0ZTktYjM1Ny00NTNjLThiNzEtNmVhMThiNzIxNGVjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUlZyRmZwTFFTMkxMM3BVME5uMjRMUEc0NklcL0tTdm9vNURwcXVRclk1UjhUb2J6cDJtakJqcUR4em93N2lSYkcifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [10.1.200.107] MIME-Version: 1.0 Return-Path: jaben.carsey@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jaben Carsey 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 > Cc: devel @ edk2 . groups . io ; Carsey, Jaben > ; Ni, Ray ; Leif Lindholm > ; Gao, Liming ; Heinrich > Schuchardt > Subject: [edk2-devel] [PATCH v3 1/1] ShellPkg/CommandLib: avoid NULL > derefence and memory leak > Importance: High >=20 > 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 gUnicodeCollati= on > gUnicodeCollation (=3D NULL) in ShellCommandRegisterCommandName(). >=20 > Furthermore CommandInit() is leaking PlatformLang if > gUnicodeCollation !=3D NULL. >=20 > Close the memory leak and use the first UnicodeCollation instance if > PlatfomLang is not defined. >=20 > Fixes: d65f2cea36d1 ("ShellPkg/CommandLib: Locate proper > UnicodeCollation > instance") > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Heinrich Schuchardt > Reviewed-by: Zhichao Gao > --- > v3 > resend as quoted-printable > --- > .../UefiShellCommandLib/UefiShellCommandLib.c | 20 +++++++++++++----- > - > 1 file changed, 14 insertions(+), 6 deletions(-) >=20 > 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; >=20 > CHAR8 *PlatformLang; >=20 >=20 >=20 > - GetEfiGlobalVariable2 (EFI_PLATFORM_LANG_VARIABLE_NAME, > (VOID**)&PlatformLang, NULL); >=20 > - if (PlatformLang =3D=3D NULL) { >=20 > - return EFI_UNSUPPORTED; >=20 > - } >=20 > - >=20 > if (gUnicodeCollation =3D=3D NULL) { >=20 > + >=20 > + GetEfiGlobalVariable2 (EFI_PLATFORM_LANG_VARIABLE_NAME, > (VOID**)&PlatformLang, NULL); >=20 > + >=20 > Status =3D gBS->LocateHandleBuffer ( >=20 > ByProtocol, >=20 > &gEfiUnicodeCollation2ProtocolGuid, >=20 > @@ -113,6 +111,14 @@ CommandInit( > continue; >=20 > } >=20 >=20 >=20 > + // >=20 > + // Without clue provided use the first Unicode Collation2 protocol= . >=20 > + // >=20 > + if (PlatformLang =3D=3D NULL) { >=20 > + gUnicodeCollation =3D Uc; >=20 > + break; >=20 > + } >=20 > + >=20 > // >=20 > // Find the best matching matching language from the supported > languages >=20 > // of Unicode Collation2 protocol. >=20 > @@ -132,7 +138,9 @@ CommandInit( > if (Handles !=3D NULL) { >=20 > FreePool (Handles); >=20 > } >=20 > - FreePool (PlatformLang); >=20 > + if (PlatformLang !=3D NULL) { >=20 > + FreePool (PlatformLang); >=20 > + } >=20 > } >=20 >=20 >=20 > return (gUnicodeCollation =3D=3D NULL) ? EFI_UNSUPPORTED : EFI_SUCCESS= ; >=20 > -- > 2.20.1 >=20 >=20 > -=3D-=3D-=3D-=3D-=3D-=3D > Groups.io Links: You receive all messages sent to this group. >=20 > View/Reply Online (#40395): https://edk2.groups.io/g/devel/message/40395 > Mute This Topic: https://groups.io/mt/31573312/1760878 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [jaben.carsey@intel.com] > -=3D-=3D-=3D-=3D-=3D-=3D