1. Patchew
  2. EDK2
  3. View series

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

Marvin Häuser posted 1 patch 2 years, 8 months ago
Failed in applying to current master (apply log)
UefiPayloadPkg/UefiPayloadEntry/MemoryAllocation.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption
Posted by Marvin Häuser 2 years, 8 months ago 
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);

 }

 

-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78895): https://edk2.groups.io/g/devel/message/78895
Mute This Topic: https://groups.io/mt/84754069/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption
Posted by Ni, Ray 2 years, 8 months ago 
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?

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);

 }

 

-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78915): https://edk2.groups.io/g/devel/message/78915
Mute This Topic: https://groups.io/mt/84754069/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption
Posted by Marvin Häuser 2 years, 8 months ago 
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);
>
>   }
>
>   
>



-=-=-=-=-=-=-=-=-=-=-=-
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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption
Posted by Guo Dong 2 years, 8 months ago 
Thanks to capture and fix this issue.

Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: Marvin Häuser <mhaeuser@posteo.de> 
Sent: Sunday, August 8, 2021 12:40 PM
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); } -- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79054): https://edk2.groups.io/g/devel/message/79054
Mute This Topic: https://groups.io/mt/84754069/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.