Re: [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption

["Marvin Häuser" <mhaeuser@posteo.de>]

On 09/08/2021 06:20, Ni, Ray wrote:
> It's so lucky that no code calls AllocatePool so the bug didn't cause real issues. (I tried to remove AllocatePool() and build still passed.)
>
> Thanks for catching the bug. Reviewed-by: Ray Ni <ray.ni@intel.com>
>
> Can you kindly share how you found this issue?

Hey Ray,

clang-tidy gave me a hand. :)
"Suspicious usage of 'sizeof(K)'; did you mean 'K'?
clang-tidy(bugprone-sizeof-expression)"

I set it up as follows (this is *not* sophisticated, just added things 
to quickly move on):
https://github.com/tianocore/edk2-staging/blob/2021-gsoc-secure-loader/compile_flags.txt

Best regards,
Marvin

>
> Thanks,
> Ray
>
> -----Original Message-----
> From: Marvin Häuser <mhaeuser@posteo.de>
> Sent: Monday, August 9, 2021 3:40 AM
> To: devel@edk2.groups.io
> Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption
>
> UefiPayloadEntry's AllocatePool() applies the "sizeof" operator to
> HOB index rather than the HOB header structure. This yields 4 Bytes
> compared to the 8 Bytes the structure header requires. Fix the call
> to allocate the required space instead.
>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
> ---
>   UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c b/UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c
> index 1204573b3e09..f3494969e5ac 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c
> +++ b/UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c
> @@ -163,7 +163,7 @@ AllocatePool (
>       return NULL;
>
>     }
>
>   
>
> -  Hob = (EFI_HOB_MEMORY_POOL *)CreateHob (EFI_HOB_TYPE_MEMORY_POOL, (UINT16)(sizeof (EFI_HOB_TYPE_MEMORY_POOL) + AllocationSize));
>
> +  Hob = (EFI_HOB_MEMORY_POOL *)CreateHob (EFI_HOB_TYPE_MEMORY_POOL, (UINT16)(sizeof (EFI_HOB_MEMORY_POOL) + AllocationSize));
>
>     return (VOID *)(Hob + 1);
>
>   }
>
>   
>