public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()
@ 2024-01-12  2:25 Guo, Gua
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 1/4] UefiPayloadPkg/Hob: " Guo, Gua
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Guo, Gua @ 2024-01-12  2:25 UTC (permalink / raw)
  To: devel
  Cc: gua.guo, Ard Biesheuvel, Gerd Hoffmann, John Mathew,
	Vincent Zimmer, Sami Mujawar

From: Gua Guo <gua.guo@intel.com>

PR: https://github.com/tianocore/edk2/pull/5252

V3
1. UefiPayloadPkg/Hob: Integer : Add error handle

2. StandaloneMmPkg/Hob: Integer Overflow in : Add error handle

3. EmbeddedPkg/Hob: Integer Overflow in CreateHob() : Add error handle

V2
1. UefiPayloadPkg/Hob: Integer : Add Reviewed-by and Authored-by

2. StandaloneMmPkg/Hob: Integer Overflow in : Add Reviewed-by and Authored-by

3. EmbeddedPkg/Hob: Integer Overflow in CreateHob() : Add Reviewed-by and Authored-by

4. MdeModulePkg/Hob: Integer Overflow in CreateHob() : Add Authored-by

V1

1. UefiPayloadPkg/Hob: Integer

2. StandaloneMmPkg/Hob: Integer Overflow in

3. EmbeddedPkg/Hob: Integer Overflow in CreateHob()

4. MdeModulePkg/Hob: Integer Overflow in CreateHob()

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>

Cc: Gerd Hoffmann <kraxel@redhat.com>

Cc: John Mathew <john.mathews@intel.com>

Cc: Vincent Zimmer <vincent.zimmer@intel.com>

Cc: Sami Mujawar <sami.mujawar@arm.com>

Gua Guo (4):
  UefiPayloadPkg/Hob: Integer Overflow in CreateHob()
  StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
  EmbeddedPkg/Hob: Integer Overflow in CreateHob()
  MdeModulePkg/Hob: Integer Overflow in CreateHob()

 EmbeddedPkg/Library/PrePiHobLib/Hob.c         | 43 +++++++++++++++++++
 MdeModulePkg/Core/Pei/Hob/Hob.c               |  2 +-
 .../Arm/StandaloneMmCoreHobLib.c              | 35 +++++++++++++++
 .../Library/PayloadEntryHobLib/Hob.c          | 43 +++++++++++++++++++
 .../FitUniversalPayloadEntry.c                |  8 ++--
 .../UefiPayloadEntry/UniversalPayloadEntry.c  |  8 ++--
 6 files changed, 132 insertions(+), 7 deletions(-)

--
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113639): https://edk2.groups.io/g/devel/message/113639
Mute This Topic: https://groups.io/mt/103675959/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 1/4] UefiPayloadPkg/Hob: Integer Overflow in CreateHob()
  2024-01-12  2:25 [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob() Guo, Gua
@ 2024-01-12  2:25 ` Guo, Gua
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: " Guo, Gua
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Guo, Gua @ 2024-01-12  2:25 UTC (permalink / raw)
  To: devel
  Cc: gua.guo, Marc Beatove, Guo Dong, Sean Rhodes, James Lu,
	John Mathew, Gerd Hoffmann

From: Gua Guo <gua.guo@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166

Fix integer overflow in various CreateHob instances.
Fixes: CVE-2022-36765

The CreateHob() function aligns the requested size to 8
performing the following operation:
```
HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
```

No checks are performed to ensure this value doesn't
overflow, and could lead to CreateHob() returning a smaller
HOB than requested, which could lead to OOB HOB accesses.

Reported-by: Marc Beatove <mbeatove@google.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Reviewed-by: Gua Guo <gua.guo@intel.com>
Cc: John Mathew <john.mathews@intel.com>
Authored-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Gua Guo <gua.guo@intel.com>
---
 .../Library/PayloadEntryHobLib/Hob.c          | 43 +++++++++++++++++++
 .../FitUniversalPayloadEntry.c                |  8 ++--
 .../UefiPayloadEntry/UniversalPayloadEntry.c  |  8 ++--
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
index 2c3acbbc19..51c2e28d7d 100644
--- a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
+++ b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
@@ -110,6 +110,13 @@ CreateHob (
 
   HandOffHob = GetHobList ();
 
+  //
+  // Check Length to avoid data overflow.
+  //
+  if (HobLength > MAX_UINT16 - 0x7) {
+    return NULL;
+  }
+
   HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
 
   FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;
@@ -160,6 +167,9 @@ BuildResourceDescriptorHob (
 
   Hob = CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof (EFI_HOB_RESOURCE_DESCRIPTOR));
   ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->ResourceType      = ResourceType;
   Hob->ResourceAttribute = ResourceAttribute;
@@ -330,6 +340,10 @@ BuildModuleHob (
     );
 
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_MODULE));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModuleGuid);
   Hob->MemoryAllocationHeader.MemoryBaseAddress = MemoryAllocationModule;
@@ -378,6 +392,11 @@ BuildGuidHob (
   ASSERT (DataLength <= (0xffff - sizeof (EFI_HOB_GUID_TYPE)));
 
   Hob = CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16)(sizeof (EFI_HOB_GUID_TYPE) + DataLength));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return NULL;
+  }
+
   CopyGuid (&Hob->Name, Guid);
   return Hob + 1;
 }
@@ -441,6 +460,10 @@ BuildFvHob (
   EFI_HOB_FIRMWARE_VOLUME  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_FV, sizeof (EFI_HOB_FIRMWARE_VOLUME));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->BaseAddress = BaseAddress;
   Hob->Length      = Length;
@@ -472,6 +495,10 @@ BuildFv2Hob (
   EFI_HOB_FIRMWARE_VOLUME2  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_FV2, sizeof (EFI_HOB_FIRMWARE_VOLUME2));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->BaseAddress = BaseAddress;
   Hob->Length      = Length;
@@ -513,6 +540,10 @@ BuildFv3Hob (
   EFI_HOB_FIRMWARE_VOLUME3  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_FV3, sizeof (EFI_HOB_FIRMWARE_VOLUME3));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->BaseAddress          = BaseAddress;
   Hob->Length               = Length;
@@ -546,6 +577,10 @@ BuildCpuHob (
   EFI_HOB_CPU  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->SizeOfMemorySpace = SizeOfMemorySpace;
   Hob->SizeOfIoSpace     = SizeOfIoSpace;
@@ -583,6 +618,10 @@ BuildStackHob (
     );
 
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_STACK));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   CopyGuid (&(Hob->AllocDescriptor.Name), &gEfiHobMemoryAllocStackGuid);
   Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
@@ -664,6 +703,10 @@ BuildMemoryAllocationHob (
     );
 
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID));
   Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
diff --git a/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.c
index d2e7df4fbe..eb0b325369 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.c
@@ -207,10 +207,12 @@ AddNewHob (
   }
 
   NewHob.Header = CreateHob (Hob->Header->HobType, Hob->Header->HobLength);
-
-  if (NewHob.Header != NULL) {
-    CopyMem (NewHob.Header + 1, Hob->Header + 1, Hob->Header->HobLength - sizeof (EFI_HOB_GENERIC_HEADER));
+  ASSERT (NewHob.Header != NULL);
+  if (NewHob.Header == NULL) {
+    return;
   }
+
+  CopyMem (NewHob.Header + 1, Hob->Header + 1, Hob->Header->HobLength - sizeof (EFI_HOB_GENERIC_HEADER));
 }
 
 /**
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
index f8939efe70..f37c00fad7 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.c
@@ -111,10 +111,12 @@ AddNewHob (
   }
 
   NewHob.Header = CreateHob (Hob->Header->HobType, Hob->Header->HobLength);
-
-  if (NewHob.Header != NULL) {
-    CopyMem (NewHob.Header + 1, Hob->Header + 1, Hob->Header->HobLength - sizeof (EFI_HOB_GENERIC_HEADER));
+  ASSERT (NewHob.Header != NULL);
+  if (NewHob.Header == NULL) {
+    return;
   }
+
+  CopyMem (NewHob.Header + 1, Hob->Header + 1, Hob->Header->HobLength - sizeof (EFI_HOB_GENERIC_HEADER));
 }
 
 /**
-- 
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113640): https://edk2.groups.io/g/devel/message/113640
Mute This Topic: https://groups.io/mt/103675960/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
  2024-01-12  2:25 [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob() Guo, Gua
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 1/4] UefiPayloadPkg/Hob: " Guo, Gua
@ 2024-01-12  2:25 ` Guo, Gua
  2024-01-12  8:56   ` Ni, Ray
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 3/4] EmbeddedPkg/Hob: " Guo, Gua
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Guo, Gua @ 2024-01-12  2:25 UTC (permalink / raw)
  To: devel
  Cc: gua.guo, Marc Beatove, Ard Biesheuvel, Sami Mujawar, Ray Ni,
	John Mathew, Gerd Hoffmann

From: Gua Guo <gua.guo@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166

Fix integer overflow in various CreateHob instances.
Fixes: CVE-2022-36765

The CreateHob() function aligns the requested size to 8
performing the following operation:
```
HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
```

No checks are performed to ensure this value doesn't
overflow, and could lead to CreateHob() returning a smaller
HOB than requested, which could lead to OOB HOB accesses.

Reported-by: Marc Beatove <mbeatove@google.com>
Reviewed-by: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: John Mathew <john.mathews@intel.com>
Authored-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Gua Guo <gua.guo@intel.com>
---
 .../Arm/StandaloneMmCoreHobLib.c              | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
index 1550e1babc..59473e28fe 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
@@ -34,6 +34,13 @@ CreateHob (
 
   HandOffHob = GetHobList ();
 
+  //
+  // Check Length to avoid data overflow.
+  //
+  if (HobLength > MAX_UINT16 - 0x7) {
+    return NULL;
+  }
+
   HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
 
   FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;
@@ -89,6 +96,10 @@ BuildModuleHob (
     );
 
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_MODULE));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModuleGuid);
   Hob->MemoryAllocationHeader.MemoryBaseAddress = MemoryAllocationModule;
@@ -129,6 +140,9 @@ BuildResourceDescriptorHob (
 
   Hob = CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof (EFI_HOB_RESOURCE_DESCRIPTOR));
   ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->ResourceType      = ResourceType;
   Hob->ResourceAttribute = ResourceAttribute;
@@ -167,6 +181,11 @@ BuildGuidHob (
   ASSERT (DataLength <= (0xffff - sizeof (EFI_HOB_GUID_TYPE)));
 
   Hob = CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16)(sizeof (EFI_HOB_GUID_TYPE) + DataLength));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return NULL;
+  }
+
   CopyGuid (&Hob->Name, Guid);
   return Hob + 1;
 }
@@ -226,6 +245,10 @@ BuildFvHob (
   EFI_HOB_FIRMWARE_VOLUME  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_FV, sizeof (EFI_HOB_FIRMWARE_VOLUME));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->BaseAddress = BaseAddress;
   Hob->Length      = Length;
@@ -255,6 +278,10 @@ BuildFv2Hob (
   EFI_HOB_FIRMWARE_VOLUME2  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_FV2, sizeof (EFI_HOB_FIRMWARE_VOLUME2));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->BaseAddress = BaseAddress;
   Hob->Length      = Length;
@@ -282,6 +309,10 @@ BuildCpuHob (
   EFI_HOB_CPU  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->SizeOfMemorySpace = SizeOfMemorySpace;
   Hob->SizeOfIoSpace     = SizeOfIoSpace;
@@ -319,6 +350,10 @@ BuildMemoryAllocationHob (
     );
 
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID));
   Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
-- 
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113641): https://edk2.groups.io/g/devel/message/113641
Mute This Topic: https://groups.io/mt/103675962/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 3/4] EmbeddedPkg/Hob: Integer Overflow in CreateHob()
  2024-01-12  2:25 [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob() Guo, Gua
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 1/4] UefiPayloadPkg/Hob: " Guo, Gua
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: " Guo, Gua
@ 2024-01-12  2:25 ` Guo, Gua
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: " Guo, Gua
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Guo, Gua @ 2024-01-12  2:25 UTC (permalink / raw)
  To: devel
  Cc: gua.guo, Marc Beatove, Leif Lindholm, Ard Biesheuvel, Abner Chang,
	John Mathew, Gerd Hoffmann

From: Gua Guo <gua.guo@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166

Fix integer overflow in various CreateHob instances.
Fixes: CVE-2022-36765

The CreateHob() function aligns the requested size to 8
performing the following operation:
```
HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
```

No checks are performed to ensure this value doesn't
overflow, and could lead to CreateHob() returning a smaller
HOB than requested, which could lead to OOB HOB accesses.

Reported-by: Marc Beatove <mbeatove@google.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Abner Chang <abner.chang@amd.com>
Cc: John Mathew <john.mathews@intel.com>
Authored-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Gua Guo <gua.guo@intel.com>
---
 EmbeddedPkg/Library/PrePiHobLib/Hob.c | 43 +++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
index 8eb175aa96..cbc35152cc 100644
--- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c
+++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
@@ -110,6 +110,13 @@ CreateHob (
 
   HandOffHob = GetHobList ();
 
+  //
+  // Check Length to avoid data overflow.
+  //
+  if (HobLength > MAX_UINT16 - 0x7) {
+    return NULL;
+  }
+
   HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
 
   FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;
@@ -160,6 +167,9 @@ BuildResourceDescriptorHob (
 
   Hob = CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof (EFI_HOB_RESOURCE_DESCRIPTOR));
   ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->ResourceType      = ResourceType;
   Hob->ResourceAttribute = ResourceAttribute;
@@ -401,6 +411,10 @@ BuildModuleHob (
     );
 
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_MODULE));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModuleGuid);
   Hob->MemoryAllocationHeader.MemoryBaseAddress = MemoryAllocationModule;
@@ -449,6 +463,11 @@ BuildGuidHob (
   ASSERT (DataLength <= (0xffff - sizeof (EFI_HOB_GUID_TYPE)));
 
   Hob = CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16)(sizeof (EFI_HOB_GUID_TYPE) + DataLength));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return NULL;
+  }
+
   CopyGuid (&Hob->Name, Guid);
   return Hob + 1;
 }
@@ -512,6 +531,10 @@ BuildFvHob (
   EFI_HOB_FIRMWARE_VOLUME  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_FV, sizeof (EFI_HOB_FIRMWARE_VOLUME));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->BaseAddress = BaseAddress;
   Hob->Length      = Length;
@@ -543,6 +566,10 @@ BuildFv2Hob (
   EFI_HOB_FIRMWARE_VOLUME2  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_FV2, sizeof (EFI_HOB_FIRMWARE_VOLUME2));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->BaseAddress = BaseAddress;
   Hob->Length      = Length;
@@ -584,6 +611,10 @@ BuildFv3Hob (
   EFI_HOB_FIRMWARE_VOLUME3  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_FV3, sizeof (EFI_HOB_FIRMWARE_VOLUME3));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->BaseAddress          = BaseAddress;
   Hob->Length               = Length;
@@ -639,6 +670,10 @@ BuildCpuHob (
   EFI_HOB_CPU  *Hob;
 
   Hob = CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   Hob->SizeOfMemorySpace = SizeOfMemorySpace;
   Hob->SizeOfIoSpace     = SizeOfIoSpace;
@@ -676,6 +711,10 @@ BuildStackHob (
     );
 
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_STACK));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   CopyGuid (&(Hob->AllocDescriptor.Name), &gEfiHobMemoryAllocStackGuid);
   Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
@@ -756,6 +795,10 @@ BuildMemoryAllocationHob (
     );
 
   Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION));
+  ASSERT (Hob != NULL);
+  if (Hob == NULL) {
+    return;
+  }
 
   ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID));
   Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
-- 
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113642): https://edk2.groups.io/g/devel/message/113642
Mute This Topic: https://groups.io/mt/103675964/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: Integer Overflow in CreateHob()
  2024-01-12  2:25 [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob() Guo, Gua
                   ` (2 preceding siblings ...)
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 3/4] EmbeddedPkg/Hob: " Guo, Gua
@ 2024-01-12  2:25 ` Guo, Gua
  2024-01-16 14:39   ` 回复: " gaoliming via groups.io
  2024-01-19  9:53 ` [edk2-devel] [PATCH v3 0/4] Bz4166: " Sami Mujawar
  2024-01-23 14:49 ` Gerd Hoffmann
  5 siblings, 1 reply; 14+ messages in thread
From: Guo, Gua @ 2024-01-12  2:25 UTC (permalink / raw)
  To: devel; +Cc: gua.guo, Marc Beatove, Liming Gao, John Mathew, Gerd Hoffmann

From: Gua Guo <gua.guo@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166

Fix integer overflow in various CreateHob instances.
Fixes: CVE-2022-36765

The CreateHob() function aligns the requested size to 8
performing the following operation:
```
HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
```

No checks are performed to ensure this value doesn't
overflow, and could lead to CreateHob() returning a smaller
HOB than requested, which could lead to OOB HOB accesses.

Reported-by: Marc Beatove <mbeatove@google.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: John Mathew <john.mathews@intel.com>
Authored-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Gua Guo <gua.guo@intel.com>
---
 MdeModulePkg/Core/Pei/Hob/Hob.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Pei/Hob/Hob.c b/MdeModulePkg/Core/Pei/Hob/Hob.c
index c4882a23cd..985da50995 100644
--- a/MdeModulePkg/Core/Pei/Hob/Hob.c
+++ b/MdeModulePkg/Core/Pei/Hob/Hob.c
@@ -85,7 +85,7 @@ PeiCreateHob (
   //
   // Check Length to avoid data overflow.
   //
-  if (0x10000 - Length <= 0x7) {
+  if (MAX_UINT16 - Length < 0x7) {
     return EFI_INVALID_PARAMETER;
   }
 
-- 
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113643): https://edk2.groups.io/g/devel/message/113643
Mute This Topic: https://groups.io/mt/103675965/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: " Guo, Gua
@ 2024-01-12  8:56   ` Ni, Ray
  2024-01-24 12:40     ` Gerd Hoffmann
  0 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2024-01-12  8:56 UTC (permalink / raw)
  To: Guo, Gua, devel@edk2.groups.io
  Cc: Marc Beatove, Ard Biesheuvel, Sami Mujawar, Mathews, John,
	Gerd Hoffmann

It's strange to me that ARM's MM env still allows modifying HOBs.

Thanks,
Ray
> -----Original Message-----
> From: Guo, Gua <gua.guo@intel.com>
> Sent: Friday, January 12, 2024 10:25 AM
> To: devel@edk2.groups.io
> Cc: Guo, Gua <gua.guo@intel.com>; Marc Beatove <mbeatove@google.com>;
> Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> <sami.mujawar@arm.com>; Ni, Ray <ray.ni@intel.com>; Mathews, John
> <john.mathews@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in
> CreateHob()
> 
> From: Gua Guo <gua.guo@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166
> 
> Fix integer overflow in various CreateHob instances.
> Fixes: CVE-2022-36765
> 
> The CreateHob() function aligns the requested size to 8
> performing the following operation:
> ```
> HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> ```
> 
> No checks are performed to ensure this value doesn't
> overflow, and could lead to CreateHob() returning a smaller
> HOB than requested, which could lead to OOB HOB accesses.
> 
> Reported-by: Marc Beatove <mbeatove@google.com>
> Reviewed-by: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: John Mathew <john.mathews@intel.com>
> Authored-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Gua Guo <gua.guo@intel.com>
> ---
>  .../Arm/StandaloneMmCoreHobLib.c              | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneM
> mCoreHobLib.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneM
> mCoreHobLib.c
> index 1550e1babc..59473e28fe 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneM
> mCoreHobLib.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneM
> mCoreHobLib.c
> @@ -34,6 +34,13 @@ CreateHob (
> 
> 
>    HandOffHob = GetHobList ();
> 
> 
> 
> +  //
> 
> +  // Check Length to avoid data overflow.
> 
> +  //
> 
> +  if (HobLength > MAX_UINT16 - 0x7) {
> 
> +    return NULL;
> 
> +  }
> 
> +
> 
>    HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> 
> 
> 
>    FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob-
> >EfiFreeMemoryBottom;
> 
> @@ -89,6 +96,10 @@ BuildModuleHob (
>      );
> 
> 
> 
>    Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof
> (EFI_HOB_MEMORY_ALLOCATION_MODULE));
> 
> +  ASSERT (Hob != NULL);
> 
> +  if (Hob == NULL) {
> 
> +    return;
> 
> +  }
> 
> 
> 
>    CopyGuid (&(Hob->MemoryAllocationHeader.Name),
> &gEfiHobMemoryAllocModuleGuid);
> 
>    Hob->MemoryAllocationHeader.MemoryBaseAddress =
> MemoryAllocationModule;
> 
> @@ -129,6 +140,9 @@ BuildResourceDescriptorHob (
> 
> 
>    Hob = CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof
> (EFI_HOB_RESOURCE_DESCRIPTOR));
> 
>    ASSERT (Hob != NULL);
> 
> +  if (Hob == NULL) {
> 
> +    return;
> 
> +  }
> 
> 
> 
>    Hob->ResourceType      = ResourceType;
> 
>    Hob->ResourceAttribute = ResourceAttribute;
> 
> @@ -167,6 +181,11 @@ BuildGuidHob (
>    ASSERT (DataLength <= (0xffff - sizeof (EFI_HOB_GUID_TYPE)));
> 
> 
> 
>    Hob = CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16)(sizeof
> (EFI_HOB_GUID_TYPE) + DataLength));
> 
> +  ASSERT (Hob != NULL);
> 
> +  if (Hob == NULL) {
> 
> +    return NULL;
> 
> +  }
> 
> +
> 
>    CopyGuid (&Hob->Name, Guid);
> 
>    return Hob + 1;
> 
>  }
> 
> @@ -226,6 +245,10 @@ BuildFvHob (
>    EFI_HOB_FIRMWARE_VOLUME  *Hob;
> 
> 
> 
>    Hob = CreateHob (EFI_HOB_TYPE_FV, sizeof
> (EFI_HOB_FIRMWARE_VOLUME));
> 
> +  ASSERT (Hob != NULL);
> 
> +  if (Hob == NULL) {
> 
> +    return;
> 
> +  }
> 
> 
> 
>    Hob->BaseAddress = BaseAddress;
> 
>    Hob->Length      = Length;
> 
> @@ -255,6 +278,10 @@ BuildFv2Hob (
>    EFI_HOB_FIRMWARE_VOLUME2  *Hob;
> 
> 
> 
>    Hob = CreateHob (EFI_HOB_TYPE_FV2, sizeof
> (EFI_HOB_FIRMWARE_VOLUME2));
> 
> +  ASSERT (Hob != NULL);
> 
> +  if (Hob == NULL) {
> 
> +    return;
> 
> +  }
> 
> 
> 
>    Hob->BaseAddress = BaseAddress;
> 
>    Hob->Length      = Length;
> 
> @@ -282,6 +309,10 @@ BuildCpuHob (
>    EFI_HOB_CPU  *Hob;
> 
> 
> 
>    Hob = CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU));
> 
> +  ASSERT (Hob != NULL);
> 
> +  if (Hob == NULL) {
> 
> +    return;
> 
> +  }
> 
> 
> 
>    Hob->SizeOfMemorySpace = SizeOfMemorySpace;
> 
>    Hob->SizeOfIoSpace     = SizeOfIoSpace;
> 
> @@ -319,6 +350,10 @@ BuildMemoryAllocationHob (
>      );
> 
> 
> 
>    Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof
> (EFI_HOB_MEMORY_ALLOCATION));
> 
> +  ASSERT (Hob != NULL);
> 
> +  if (Hob == NULL) {
> 
> +    return;
> 
> +  }
> 
> 
> 
>    ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID));
> 
>    Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress;
> 
> --
> 2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113699): https://edk2.groups.io/g/devel/message/113699
Mute This Topic: https://groups.io/mt/103675962/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* 回复: [edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: Integer Overflow in CreateHob()
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: " Guo, Gua
@ 2024-01-16 14:39   ` gaoliming via groups.io
  0 siblings, 0 replies; 14+ messages in thread
From: gaoliming via groups.io @ 2024-01-16 14:39 UTC (permalink / raw)
  To: devel, gua.guo
  Cc: 'Marc Beatove', 'John Mathew',
	'Gerd Hoffmann'

Gua:
 I think new code logic is same to old one. Can you point what difference
here?

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Guo, Gua
> 发送时间: 2024年1月12日 10:25
> 收件人: devel@edk2.groups.io
> 抄送: gua.guo@intel.com; Marc Beatove <mbeatove@google.com>; Liming
> Gao <gaoliming@byosoft.com.cn>; John Mathew <john.mathews@intel.com>;
> Gerd Hoffmann <kraxel@redhat.com>
> 主题: [edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: Integer Overflow in
> CreateHob()
> 
> From: Gua Guo <gua.guo@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166
> 
> Fix integer overflow in various CreateHob instances.
> Fixes: CVE-2022-36765
> 
> The CreateHob() function aligns the requested size to 8
> performing the following operation:
> ```
> HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> ```
> 
> No checks are performed to ensure this value doesn't
> overflow, and could lead to CreateHob() returning a smaller
> HOB than requested, which could lead to OOB HOB accesses.
> 
> Reported-by: Marc Beatove <mbeatove@google.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: John Mathew <john.mathews@intel.com>
> Authored-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Gua Guo <gua.guo@intel.com>
> ---
>  MdeModulePkg/Core/Pei/Hob/Hob.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/Hob/Hob.c
> b/MdeModulePkg/Core/Pei/Hob/Hob.c
> index c4882a23cd..985da50995 100644
> --- a/MdeModulePkg/Core/Pei/Hob/Hob.c
> +++ b/MdeModulePkg/Core/Pei/Hob/Hob.c
> @@ -85,7 +85,7 @@ PeiCreateHob (
>    //
> 
>    // Check Length to avoid data overflow.
> 
>    //
> 
> -  if (0x10000 - Length <= 0x7) {
> 
> +  if (MAX_UINT16 - Length < 0x7) {
> 
>      return EFI_INVALID_PARAMETER;
> 
>    }
> 
> 
> 
> --
> 2.39.2.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#113643):
> https://edk2.groups.io/g/devel/message/113643
> Mute This Topic: https://groups.io/mt/103675965/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113905): https://edk2.groups.io/g/devel/message/113905
Mute This Topic: https://groups.io/mt/103762835/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()
  2024-01-12  2:25 [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob() Guo, Gua
                   ` (3 preceding siblings ...)
  2024-01-12  2:25 ` [edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: " Guo, Gua
@ 2024-01-19  9:53 ` Sami Mujawar
  2024-01-23 14:49 ` Gerd Hoffmann
  5 siblings, 0 replies; 14+ messages in thread
From: Sami Mujawar @ 2024-01-19  9:53 UTC (permalink / raw)
  To: gua.guo@intel.com, devel@edk2.groups.io
  Cc: Ard Biesheuvel, Gerd Hoffmann, John Mathew, Vincent Zimmer,
	quic_llindhol@quicinc.com

Hi Gua,

I don’t think handling the error one level up (i.e. only in the calling function) solves the problem in entirety, can you check please?
Example, now the crash can happen in BuildGuidDataHob() see https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Library/PrePiHobLib/Hob.c#L488-L490
I believe such cases are at other places as well.

I think it may be better to introduce a Panic() hander to fix this properly.

Regards,

Sami Mujawar

On 12/01/2024, 02:25, "gua.guo@intel.com <mailto:gua.guo@intel.com>" <gua.guo@intel.com <mailto:gua.guo@intel.com>> wrote:


From: Gua Guo <gua.guo@intel.com <mailto:gua.guo@intel.com>>


PR: https://github.com/tianocore/edk2/pull/5252 <https://github.com/tianocore/edk2/pull/5252>


V3
1. UefiPayloadPkg/Hob: Integer : Add error handle


2. StandaloneMmPkg/Hob: Integer Overflow in : Add error handle


3. EmbeddedPkg/Hob: Integer Overflow in CreateHob() : Add error handle


V2
1. UefiPayloadPkg/Hob: Integer : Add Reviewed-by and Authored-by


2. StandaloneMmPkg/Hob: Integer Overflow in : Add Reviewed-by and Authored-by


3. EmbeddedPkg/Hob: Integer Overflow in CreateHob() : Add Reviewed-by and Authored-by


4. MdeModulePkg/Hob: Integer Overflow in CreateHob() : Add Authored-by


V1


1. UefiPayloadPkg/Hob: Integer


2. StandaloneMmPkg/Hob: Integer Overflow in


3. EmbeddedPkg/Hob: Integer Overflow in CreateHob()


4. MdeModulePkg/Hob: Integer Overflow in CreateHob()


Cc: Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>


Cc: Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>


Cc: John Mathew <john.mathews@intel.com <mailto:john.mathews@intel.com>>


Cc: Vincent Zimmer <vincent.zimmer@intel.com <mailto:vincent.zimmer@intel.com>>


Cc: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>


Gua Guo (4):
UefiPayloadPkg/Hob: Integer Overflow in CreateHob()
StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
EmbeddedPkg/Hob: Integer Overflow in CreateHob()
MdeModulePkg/Hob: Integer Overflow in CreateHob()


EmbeddedPkg/Library/PrePiHobLib/Hob.c | 43 +++++++++++++++++++
MdeModulePkg/Core/Pei/Hob/Hob.c | 2 +-
.../Arm/StandaloneMmCoreHobLib.c | 35 +++++++++++++++
.../Library/PayloadEntryHobLib/Hob.c | 43 +++++++++++++++++++
.../FitUniversalPayloadEntry.c | 8 ++--
.../UefiPayloadEntry/UniversalPayloadEntry.c | 8 ++--
6 files changed, 132 insertions(+), 7 deletions(-)


--
2.39.2.windows.1





IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114038): https://edk2.groups.io/g/devel/message/114038
Mute This Topic: https://groups.io/mt/103675959/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()
  2024-01-12  2:25 [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob() Guo, Gua
                   ` (4 preceding siblings ...)
  2024-01-19  9:53 ` [edk2-devel] [PATCH v3 0/4] Bz4166: " Sami Mujawar
@ 2024-01-23 14:49 ` Gerd Hoffmann
  2024-01-23 15:16   ` Guo, Gua
  5 siblings, 1 reply; 14+ messages in thread
From: Gerd Hoffmann @ 2024-01-23 14:49 UTC (permalink / raw)
  To: gua.guo
  Cc: devel, Ard Biesheuvel, John Mathew, Vincent Zimmer, Sami Mujawar,
	jmaloy

On Fri, Jan 12, 2024 at 10:25:16AM +0800, gua.guo@intel.com wrote:
> From: Gua Guo <gua.guo@intel.com>
> 
> PR: https://github.com/tianocore/edk2/pull/5252

> Gua Guo (4):
>   UefiPayloadPkg/Hob: Integer Overflow in CreateHob()
>   StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
>   EmbeddedPkg/Hob: Integer Overflow in CreateHob()
>   MdeModulePkg/Hob: Integer Overflow in CreateHob()

Ping.  What is the status here?

Patch 1/4 has been merged (commit 59f024c76ee5), the other tree patches
are missing still.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114204): https://edk2.groups.io/g/devel/message/114204
Mute This Topic: https://groups.io/mt/103675959/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()
  2024-01-23 14:49 ` Gerd Hoffmann
@ 2024-01-23 15:16   ` Guo, Gua
  2024-01-24 12:48     ` Gerd Hoffmann
  0 siblings, 1 reply; 14+ messages in thread
From: Guo, Gua @ 2024-01-23 15:16 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Mathews, John,
	Zimmer, Vincent, Sami Mujawar, jmaloy@redhat.com

For MdeModulePkg, I think no need to change because no any logic change.

For StandaloneMmPkg and EmbeddedPkg
- Don't have enough abilities to close Sami Mujawar and Ni Ray open currently, so hold on the change until I find how to introduce Panic. So give up these two packages patch currently.

-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com> 
Sent: Tuesday, January 23, 2024 10:50 PM
To: Guo, Gua <gua.guo@intel.com>
Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Mathews, John <john.mathews@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>; Sami Mujawar <sami.mujawar@arm.com>; jmaloy@redhat.com
Subject: Re: [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

On Fri, Jan 12, 2024 at 10:25:16AM +0800, gua.guo@intel.com wrote:
> From: Gua Guo <gua.guo@intel.com>
> 
> PR: https://github.com/tianocore/edk2/pull/5252

> Gua Guo (4):
>   UefiPayloadPkg/Hob: Integer Overflow in CreateHob()
>   StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
>   EmbeddedPkg/Hob: Integer Overflow in CreateHob()
>   MdeModulePkg/Hob: Integer Overflow in CreateHob()

Ping.  What is the status here?

Patch 1/4 has been merged (commit 59f024c76ee5), the other tree patches are missing still.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114205): https://edk2.groups.io/g/devel/message/114205
Mute This Topic: https://groups.io/mt/103675959/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
  2024-01-12  8:56   ` Ni, Ray
@ 2024-01-24 12:40     ` Gerd Hoffmann
  2024-01-25  1:33       ` Ni, Ray
  0 siblings, 1 reply; 14+ messages in thread
From: Gerd Hoffmann @ 2024-01-24 12:40 UTC (permalink / raw)
  To: Ni, Ray
  Cc: Guo, Gua, devel@edk2.groups.io, Marc Beatove, Ard Biesheuvel,
	Sami Mujawar, Mathews, John

On Fri, Jan 12, 2024 at 08:56:02AM +0000, Ni, Ray wrote:
> It's strange to me that ARM's MM env still allows modifying HOBs.

Yes.

But fixing that is beyond the scope of this patch, which just
fixes the integer overflow in CreateHob().

Can we please move forward and get the remaining CreateHob()
fixes merged?

thanks & take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114283): https://edk2.groups.io/g/devel/message/114283
Mute This Topic: https://groups.io/mt/103675962/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()
  2024-01-23 15:16   ` Guo, Gua
@ 2024-01-24 12:48     ` Gerd Hoffmann
  2024-01-25  8:08       ` Guo, Gua
  0 siblings, 1 reply; 14+ messages in thread
From: Gerd Hoffmann @ 2024-01-24 12:48 UTC (permalink / raw)
  To: Guo, Gua
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Mathews, John,
	Zimmer, Vincent, Sami Mujawar, jmaloy@redhat.com

On Tue, Jan 23, 2024 at 03:16:32PM +0000, Guo, Gua wrote:
> For MdeModulePkg, I think no need to change because no any logic change.
> 
> For StandaloneMmPkg and EmbeddedPkg
> - Don't have enough abilities to close Sami Mujawar and Ni Ray open currently, so hold on the change until I find how to introduce Panic. So give up these two packages patch currently.

On StandaloneMmPkg: I think the patch is fine, I've replied in that
subthread.

On EmbeddedPkg:  I think the BuildGuidDataHob() callsites need review
whenever they do:

  (a) check the return value properly, or
  (b) allocate a fixed size HOB so the new check in CreateHob() can't
      fail.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114284): https://edk2.groups.io/g/devel/message/114284
Mute This Topic: https://groups.io/mt/103675959/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()
  2024-01-24 12:40     ` Gerd Hoffmann
@ 2024-01-25  1:33       ` Ni, Ray
  0 siblings, 0 replies; 14+ messages in thread
From: Ni, Ray @ 2024-01-25  1:33 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Guo, Gua, devel@edk2.groups.io, Marc Beatove, Ard Biesheuvel,
	Sami Mujawar, Mathews, John

Agree.
Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Wednesday, January 24, 2024 8:41 PM
> To: Ni, Ray <ray.ni@intel.com>
> Cc: Guo, Gua <gua.guo@intel.com>; devel@edk2.groups.io; Marc Beatove
> <mbeatove@google.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Sami Mujawar <sami.mujawar@arm.com>; Mathews, John
> <john.mathews@intel.com>
> Subject: Re: RE: [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in
> CreateHob()
> 
> On Fri, Jan 12, 2024 at 08:56:02AM +0000, Ni, Ray wrote:
> > It's strange to me that ARM's MM env still allows modifying HOBs.
> 
> Yes.
> 
> But fixing that is beyond the scope of this patch, which just
> fixes the integer overflow in CreateHob().
> 
> Can we please move forward and get the remaining CreateHob()
> fixes merged?
> 
> thanks & take care,
>   Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114359): https://edk2.groups.io/g/devel/message/114359
Mute This Topic: https://groups.io/mt/103675962/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()
  2024-01-24 12:48     ` Gerd Hoffmann
@ 2024-01-25  8:08       ` Guo, Gua
  0 siblings, 0 replies; 14+ messages in thread
From: Guo, Gua @ 2024-01-25  8:08 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Mathews, John,
	Zimmer, Vincent, Sami Mujawar, jmaloy@redhat.com

Hi @Gerd Hoffmann

It's PR https://github.com/tianocore/edk2/pull/5298 if no more concern received, will merge it tomorrow morning.

Thanks,
Gua

-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com> 
Sent: Wednesday, January 24, 2024 8:48 PM
To: Guo, Gua <gua.guo@intel.com>
Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Mathews, John <john.mathews@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>; Sami Mujawar <sami.mujawar@arm.com>; jmaloy@redhat.com
Subject: Re: RE: [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

On Tue, Jan 23, 2024 at 03:16:32PM +0000, Guo, Gua wrote:
> For MdeModulePkg, I think no need to change because no any logic change.
> 
> For StandaloneMmPkg and EmbeddedPkg
> - Don't have enough abilities to close Sami Mujawar and Ni Ray open currently, so hold on the change until I find how to introduce Panic. So give up these two packages patch currently.

On StandaloneMmPkg: I think the patch is fine, I've replied in that subthread.

On EmbeddedPkg:  I think the BuildGuidDataHob() callsites need review whenever they do:

  (a) check the return value properly, or
  (b) allocate a fixed size HOB so the new check in CreateHob() can't
      fail.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114370): https://edk2.groups.io/g/devel/message/114370
Mute This Topic: https://groups.io/mt/103675959/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-01-25  8:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-12  2:25 [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob() Guo, Gua
2024-01-12  2:25 ` [edk2-devel] [PATCH v3 1/4] UefiPayloadPkg/Hob: " Guo, Gua
2024-01-12  2:25 ` [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: " Guo, Gua
2024-01-12  8:56   ` Ni, Ray
2024-01-24 12:40     ` Gerd Hoffmann
2024-01-25  1:33       ` Ni, Ray
2024-01-12  2:25 ` [edk2-devel] [PATCH v3 3/4] EmbeddedPkg/Hob: " Guo, Gua
2024-01-12  2:25 ` [edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: " Guo, Gua
2024-01-16 14:39   ` 回复: " gaoliming via groups.io
2024-01-19  9:53 ` [edk2-devel] [PATCH v3 0/4] Bz4166: " Sami Mujawar
2024-01-23 14:49 ` Gerd Hoffmann
2024-01-23 15:16   ` Guo, Gua
2024-01-24 12:48     ` Gerd Hoffmann
2024-01-25  8:08       ` Guo, Gua

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox