Reviewed-by: Ruiyu Ni <ruiyu...@intel.com>
Thanks/Ray
> -----Original Message-----
> From: Wang, Jian J
> Sent: Tuesday, November 7, 2017 1:11 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.car...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>;
> Bi, Dandan <dandan...@intel.com>
> Subject: [PATCH v2 2/3] ShellPkg: Fix misuses of AllocateCopyPool
>
> > v2:
> > a. Use ReallocatePool instead of allocating then copying wherever
> applicable
>
> AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of
> memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger
> than size of "Buffer", heap memory overflow occurs during copy.
>
> One solution is to allocate pool first then copy the necessary bytes to new
> memory. Another is using ReallocatePool instead if old buffer will be freed
> on spot.
>
> Cc: Jaben Carsey <jaben.car...@intel.com>
> Cc: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Bi Dandan <dandan...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.w...@intel.com>
> ---
> ShellPkg/Application/Shell/Shell.c | 4 +++-
> ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c | 7
> +++++--
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 5471930ba1..656206fdce 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -1646,7 +1646,7 @@ ShellConvertVariables (
> //
> // now do the replacements...
> //
> - NewCommandLine1 = AllocateCopyPool(NewSize, OriginalCommandLine);
> + NewCommandLine1 = AllocateZeroPool (NewSize);
> NewCommandLine2 = AllocateZeroPool(NewSize);
> ItemTemp = AllocateZeroPool(ItemSize+(2*sizeof(CHAR16)));
> if (NewCommandLine1 == NULL || NewCommandLine2 == NULL ||
> ItemTemp == NULL) {
> @@ -1655,6 +1655,8 @@ ShellConvertVariables (
> SHELL_FREE_NON_NULL(ItemTemp);
> return (NULL);
> }
> + CopyMem (NewCommandLine1, OriginalCommandLine, StrSize
> (OriginalCommandLine));
> +
> for (MasterEnvList = EfiShellGetEnv(NULL)
> ; MasterEnvList != NULL && *MasterEnvList != CHAR_NULL
> ; MasterEnvList += StrLen(MasterEnvList) + 1
> diff --git
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> index 1122c89b8b..ee3db63358 100644
> ---
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> +++
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> @@ -143,10 +143,11 @@ UpdateOptionalData(
> OriginalOptionDataSize += (*(UINT16*)(OriginalData + sizeof(UINT32)));
> OriginalOptionDataSize -= OriginalSize;
> NewSize = OriginalSize - OriginalOptionDataSize + DataSize;
> - NewData = AllocateCopyPool(NewSize, OriginalData);
> + NewData = AllocatePool(NewSize);
> if (NewData == NULL) {
> Status = EFI_OUT_OF_RESOURCES;
> } else {
> + CopyMem (NewData, OriginalData, OriginalSize - OriginalOptionDataSize);
> CopyMem(NewData + OriginalSize - OriginalOptionDataSize, Data,
> DataSize);
> }
> }
> @@ -1120,11 +1121,13 @@ BcfgAddOpt(
> // Now we know how many EFI_INPUT_KEY structs we need to attach to
> the end of the EFI_KEY_OPTION struct.
> // Re-allocate with the added information.
> //
> - KeyOptionBuffer = AllocateCopyPool(sizeof(EFI_KEY_OPTION) +
> (sizeof(EFI_INPUT_KEY) * NewKeyOption.KeyData.Options.InputKeyCount),
> &NewKeyOption);
> + KeyOptionBuffer = AllocatePool (sizeof(EFI_KEY_OPTION) +
> (sizeof(EFI_INPUT_KEY) * NewKeyOption.KeyData.Options.InputKeyCount));
> if (KeyOptionBuffer == NULL) {
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_MEM),
> gShellBcfgHiiHandle, L"bcfg");
> ShellStatus = SHELL_OUT_OF_RESOURCES;
> + return ShellStatus;
> }
> + CopyMem (KeyOptionBuffer, &NewKeyOption,
> sizeof(EFI_KEY_OPTION));
> }
> for (LoopCounter = 0 ; ShellStatus == SHELL_SUCCESS && LoopCounter <
> NewKeyOption.KeyData.Options.InputKeyCount; LoopCounter++) {
> //
> --
> 2.14.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
