[edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

[edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

[Guo, Gua]

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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 1/4] UefiPayloadPkg/Hob: Integer Overflow in CreateHob()

[Guo, Gua]

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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

[Guo, Gua]

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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 3/4] EmbeddedPkg/Hob: Integer Overflow in CreateHob()

[Guo, Gua]

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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: Integer Overflow in CreateHob()

[Guo, Gua]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

[Ni, Ray]

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]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] [PATCH v3 4/4] MdeModulePkg/Hob: Integer Overflow in CreateHob()

[gaoliming via groups.io]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

[Sami Mujawar]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

[Gerd Hoffmann]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

[Guo, Gua]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

[Gerd Hoffmann]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

[Gerd Hoffmann]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

[Sami Mujawar]

Hi All,

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 24/01/2024, 12:41, "Gerd Hoffmann" <kraxel@redhat.com <mailto:kraxel@redhat.com>> wrote:


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.
[SAMI] We are investigating the possibility of removing the HOB modification code for Arm. However, this may involve changes at multiple places in the software stack.
Therefore, it may not be possible to immediately remove that support. 
[/SAMI]

Yes.


But fixing that is beyond the scope of this patch, which just
[SAMI] I agree, it is beyond the scope of this patch.

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 (#114313): https://edk2.groups.io/g/devel/message/114313
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

[Ni, Ray]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/4] Bz4166: Integer Overflow in CreateHob()

[Guo, Gua]

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]
-=-=-=-=-=-=-=-=-=-=-=-