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@...>

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

}