[edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption

[Marvin Häuser]

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 at 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 at posteo.de>
> Sent: Monday, August 9, 2021 3:40 AM
> To: devel at edk2.groups.io
> Cc: Dong, Guo <guo.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Ma, Maurice <maurice.ma at intel.com>; You, Benjamin <benjamin.you at intel.com>; Vitaly Cheptsov <vit9696 at 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 at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Maurice Ma <maurice.ma at intel.com>
> Cc: Benjamin You <benjamin.you at intel.com>
> Cc: Vitaly Cheptsov <vit9696 at protonmail.com>
> Signed-off-by: Marvin Häuser <mhaeuser at 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);
>
>   }
>
>   
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78921): https://edk2.groups.io/g/devel/message/78921
Mute This Topic: https://groups.io/mt/84754069/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-