Re: [edk2-devel] [PATCH 1/1] ShellPkg/CommandLib: avoid NULL derefence and memory leak

["Gao, Zhichao" <zhichao.gao@intel.com>]

The patch code looks good to me.
But while I apply this patch, it will shows "error: corrupt patch at line 24" and " error: could not build fake ancestor". Maybe you direct change the patch file and remove some blank lines.
For Uefi BdsDxe driver, the variable "PlatfomLang" would be initialize all the time. But maybe other manufacturers may have their own Bds solution and do not set this variable. This patch makes the Shell app more compatible.

Without the patch warning.
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Heinrich Schuchardt
> Sent: Friday, April 5, 2019 9:29 AM
> To: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Cc: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Heinrich Schuchardt
> <xypron.glpk@gmx.de>
> Subject: [edk2-devel] [PATCH 1/1] ShellPkg/CommandLib: avoid NULL
> derefence and memory leak
> 
> 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.glpk@gmx.de>
> ---
>  .../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 (#38597): https://edk2.groups.io/g/devel/message/38597
> Mute This Topic: https://groups.io/mt/30920004/1768756
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [zhichao.gao@intel.com] -=-=-=-=-=-=