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

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

[Guo, Gua]

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

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>

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                      | 7 +++++++
 MdeModulePkg/Core/Pei/Hob/Hob.c                            | 2 +-
 .../StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c    | 7 +++++++
 UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c            | 7 +++++++
 4 files changed, 22 insertions(+), 1 deletion(-)

--
2.39.2.windows.1



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

[edk2-devel] [PATCH v2 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>
---
 UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c b/UefiPayloadPkg/Library/PayloadEntryHobLib/Hob.c
index 2c3acbbc19..39f07d1964 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;
-- 
2.39.2.windows.1



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

[edk2-devel] [PATCH v2 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>
---
 .../StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c    | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
index 1550e1babc..bb8426dc0a 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;
-- 
2.39.2.windows.1



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

[edk2-devel] [PATCH v2 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 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
index 8eb175aa96..1fe3ea93e4 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;
-- 
2.39.2.windows.1



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

[edk2-devel] [PATCH v2 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 (#113610): https://edk2.groups.io/g/devel/message/113610
Mute This Topic: https://groups.io/mt/103658964/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Gerd Hoffmann]

On Thu, Jan 11, 2024 at 05:14:35PM +0800, gua.guo@intel.com wrote:
> From: Gua Guo <gua.guo@intel.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()

Series:
Acked-by: Gerd Hoffmann <kraxel@redhat.com>



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

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

[Guo, Gua]

Hi @Gao, Liming

I may need to get your help to check this change when you're available.
If it's fine for you from MdeModulePkg. I think we can merge this PR.

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

Thanks,
Gua

-----Original Message-----
From: Guo, Gua <gua.guo@intel.com> 
Sent: Thursday, January 11, 2024 5:15 PM
To: devel@edk2.groups.io
Cc: Guo, Gua <gua.guo@intel.com>; Marc Beatove <mbeatove@google.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Mathews, John <john.mathews@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
Subject: [PATCH v2 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 (#113617): https://edk2.groups.io/g/devel/message/113617
Mute This Topic: https://groups.io/mt/103658964/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Sami Mujawar]

Hi Gua,

Thank you for this patch.
Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 11/01/2024, 09:15, "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>>


REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166 <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 <mailto:mbeatove@google.com>>
Reviewed-by: Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>
Cc: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>
Cc: Ray Ni <ray.ni@intel.com <mailto:ray.ni@intel.com>>
Cc: John Mathew <john.mathews@intel.com <mailto:john.mathews@intel.com>>
Authored-by: Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>
Signed-off-by: Gua Guo <gua.guo@intel.com <mailto:gua.guo@intel.com>>
---
.../StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c | 7 +++++++
1 file changed, 7 insertions(+)


diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
index 1550e1babc..bb8426dc0a 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;
[SAMI] Although this fix is correct, I think it shifts the problem somewhere else. 
If the above condition occurs, a NULL is returned. A quick scan reveals that the calling functions do not check the returned value before use.
e.g. https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170
There are multiple such places where the calling functions do not check the pointer returned by CreateHob(). 
I believe a similar situation can happen for the other patches in this series.
[/SAMI]

+ }


+


HobLength = (UINT16)((HobLength + 0x7) & (~0x7));






FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;


-- 
2.39.2.windows.1







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

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

[Guo, Gua]

You mean we need to add below error handle after all callers ?

Hob = CreateHob (...)
ASSERT (Hob != NULL); <---------------- Here


Thanks,
Gua
-----Original Message-----
From: Sami Mujawar <Sami.Mujawar@arm.com> 
Sent: Thursday, January 11, 2024 10:06 PM
To: Guo, Gua <gua.guo@intel.com>; devel@edk2.groups.io
Cc: Marc Beatove <mbeatove@google.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Ni, Ray <ray.ni@intel.com>; Mathews, John <john.mathews@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; nd <nd@arm.com>
Subject: Re: [PATCH v2 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

Hi Gua,

Thank you for this patch.
Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 11/01/2024, 09:15, "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>>


REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166 <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 <mailto:mbeatove@google.com>>
Reviewed-by: Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>
Cc: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>
Cc: Ray Ni <ray.ni@intel.com <mailto:ray.ni@intel.com>>
Cc: John Mathew <john.mathews@intel.com <mailto:john.mathews@intel.com>>
Authored-by: Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>
Signed-off-by: Gua Guo <gua.guo@intel.com <mailto:gua.guo@intel.com>>
---
.../StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c | 7 +++++++
1 file changed, 7 insertions(+)


diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
index 1550e1babc..bb8426dc0a 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCor
+++ eHobLib.c
@@ -34,6 +34,13 @@ CreateHob (




HandOffHob = GetHobList ();






+ //


+ // Check Length to avoid data overflow.


+ //


+ if (HobLength > MAX_UINT16 - 0x7) {


+ return NULL;
[SAMI] Although this fix is correct, I think it shifts the problem somewhere else. 
If the above condition occurs, a NULL is returned. A quick scan reveals that the calling functions do not check the returned value before use.
e.g. https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170
There are multiple such places where the calling functions do not check the pointer returned by CreateHob(). 
I believe a similar situation can happen for the other patches in this series.
[/SAMI]

+ }


+


HobLength = (UINT16)((HobLength + 0x7) & (~0x7));






FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;


--
2.39.2.windows.1







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

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

[Sami Mujawar]

Hi Gua,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar
On 11/01/2024, 14:19, "Guo, Gua" <gua.guo@intel.com <mailto:gua.guo@intel.com>> wrote:


You mean we need to add below error handle after all callers ?


Hob = CreateHob (...)
ASSERT (Hob != NULL); <---------------- Here
[SAMI] That would certainly help catch issues in the debug builds. But the problem with asserts is, they vanish in release builds. 
I think we should consider adding appropriate error handling in the calling functions to make sure that they do not result in a crash.
[/SAMI]




Thanks,
Gua
-----Original Message-----
From: Sami Mujawar <Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>> 
Sent: Thursday, January 11, 2024 10:06 PM
To: Guo, Gua <gua.guo@intel.com <mailto:gua.guo@intel.com>>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>
Cc: Marc Beatove <mbeatove@google.com <mailto:mbeatove@google.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>; Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Mathews, John <john.mathews@intel.com <mailto:john.mathews@intel.com>>; Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>; nd <nd@arm.com <mailto:nd@arm.com>>
Subject: Re: [PATCH v2 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()


Hi Gua,


Thank you for this patch.
Please see my response inline marked [SAMI].


Regards,


Sami Mujawar


On 11/01/2024, 09:15, "gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>>" <gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>>> wrote:




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




REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166 <https://bugzilla.tianocore.org/show_bug.cgi?id=4166> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166&gt;>




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




diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
index 1550e1babc..bb8426dc0a 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCor
+++ eHobLib.c
@@ -34,6 +34,13 @@ CreateHob (








HandOffHob = GetHobList ();












+ //




+ // Check Length to avoid data overflow.




+ //




+ if (HobLength > MAX_UINT16 - 0x7) {




+ return NULL;
[SAMI] Although this fix is correct, I think it shifts the problem somewhere else. 
If the above condition occurs, a NULL is returned. A quick scan reveals that the calling functions do not check the returned value before use.
e.g. https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170 <https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170>
There are multiple such places where the calling functions do not check the pointer returned by CreateHob(). 
I believe a similar situation can happen for the other patches in this series.
[/SAMI]


+ }




+




HobLength = (UINT16)((HobLength + 0x7) & (~0x7));












FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;




--
2.39.2.windows.1















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

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

[Guo, Gua]

Maybe I can add error handle but they will have several case need to do if it's fine.
It maybe increasing some BIOS size.

    Error Handle     Error Handle        Error Handle                       Error Handle
A ----------------> B -----------------> C ------------------> CreateHob ----------------> return NULL

All caller chain may need to add it if we really want to prevent it on release.

Thanks,
Gua
-----Original Message-----
From: Sami Mujawar <Sami.Mujawar@arm.com> 
Sent: Thursday, January 11, 2024 11:02 PM
To: Guo, Gua <gua.guo@intel.com>; devel@edk2.groups.io
Cc: Marc Beatove <mbeatove@google.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Ni, Ray <ray.ni@intel.com>; Mathews, John <john.mathews@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; nd <nd@arm.com>
Subject: Re: [PATCH v2 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()

Hi Gua,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar
On 11/01/2024, 14:19, "Guo, Gua" <gua.guo@intel.com <mailto:gua.guo@intel.com>> wrote:


You mean we need to add below error handle after all callers ?


Hob = CreateHob (...)
ASSERT (Hob != NULL); <---------------- Here [SAMI] That would certainly help catch issues in the debug builds. But the problem with asserts is, they vanish in release builds. 
I think we should consider adding appropriate error handling in the calling functions to make sure that they do not result in a crash.
[/SAMI]




Thanks,
Gua
-----Original Message-----
From: Sami Mujawar <Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>>
Sent: Thursday, January 11, 2024 10:06 PM
To: Guo, Gua <gua.guo@intel.com <mailto:gua.guo@intel.com>>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>
Cc: Marc Beatove <mbeatove@google.com <mailto:mbeatove@google.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>; Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Mathews, John <john.mathews@intel.com <mailto:john.mathews@intel.com>>; Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>; nd <nd@arm.com <mailto:nd@arm.com>>
Subject: Re: [PATCH v2 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()


Hi Gua,


Thank you for this patch.
Please see my response inline marked [SAMI].


Regards,


Sami Mujawar


On 11/01/2024, 09:15, "gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>>" <gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>>> wrote:




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




REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166 <https://bugzilla.tianocore.org/show_bug.cgi?id=4166> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166&gt;>




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




diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
index 1550e1babc..bb8426dc0a 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCor
+++ eHobLib.c
@@ -34,6 +34,13 @@ CreateHob (








HandOffHob = GetHobList ();












+ //




+ // Check Length to avoid data overflow.




+ //




+ if (HobLength > MAX_UINT16 - 0x7) {




+ return NULL;
[SAMI] Although this fix is correct, I think it shifts the problem somewhere else. 
If the above condition occurs, a NULL is returned. A quick scan reveals that the calling functions do not check the returned value before use.
e.g. https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170 <https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170>
There are multiple such places where the calling functions do not check the pointer returned by CreateHob(). 
I believe a similar situation can happen for the other patches in this series.
[/SAMI]


+ }




+




HobLength = (UINT16)((HobLength + 0x7) & (~0x7));












FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;




--
2.39.2.windows.1















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

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

[Sami Mujawar]

Hi Gua,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 11/01/2024, 15:19, "Guo, Gua" <gua.guo@intel.com <mailto:gua.guo@intel.com>> wrote:


Maybe I can add error handle but they will have several case need to do if it's fine.
It maybe increasing some BIOS size.


Error Handle Error Handle Error Handle Error Handle
A ----------------> B -----------------> C ------------------> CreateHob ----------------> return NULL


All caller chain may need to add it if we really want to prevent it on release.
[SAMI] As I understand, in most cases such an error may not be recoverable. In such cases, would it make sense for the ASSERT() implementation to call a Panic() in release builds? 
Adding a Panic() API has been discussed before at https://edk2.groups.io/g/devel/message/86354.
[/SAMI]

Thanks,
Gua
-----Original Message-----
From: Sami Mujawar <Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>> 
Sent: Thursday, January 11, 2024 11:02 PM
To: Guo, Gua <gua.guo@intel.com <mailto:gua.guo@intel.com>>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>
Cc: Marc Beatove <mbeatove@google.com <mailto:mbeatove@google.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>; Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Mathews, John <john.mathews@intel.com <mailto:john.mathews@intel.com>>; Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>; nd <nd@arm.com <mailto:nd@arm.com>>
Subject: Re: [PATCH v2 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()


Hi Gua,


Please find my response inline marked [SAMI].


Regards,


Sami Mujawar
On 11/01/2024, 14:19, "Guo, Gua" <gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>>> wrote:




You mean we need to add below error handle after all callers ?




Hob = CreateHob (...)
ASSERT (Hob != NULL); <---------------- Here [SAMI] That would certainly help catch issues in the debug builds. But the problem with asserts is, they vanish in release builds. 
I think we should consider adding appropriate error handling in the calling functions to make sure that they do not result in a crash.
[/SAMI]








Thanks,
Gua
-----Original Message-----
From: Sami Mujawar <Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com> <mailto:Sami.Mujawar@arm.com <mailto:Sami.Mujawar@arm.com>>>
Sent: Thursday, January 11, 2024 10:06 PM
To: Guo, Gua <gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>>>; devel@edk2.groups.io <mailto:devel@edk2.groups.io> <mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>>
Cc: Marc Beatove <mbeatove@google.com <mailto:mbeatove@google.com> <mailto:mbeatove@google.com <mailto:mbeatove@google.com>>>; Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org> <mailto:ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>>; Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com> <mailto:ray.ni@intel.com <mailto:ray.ni@intel.com>>>; Mathews, John <john.mathews@intel.com <mailto:john.mathews@intel.com> <mailto:john.mathews@intel.com <mailto:john.mathews@intel.com>>>; Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com> <mailto:kraxel@redhat.com <mailto:kraxel@redhat.com>>>; nd <nd@arm.com <mailto:nd@arm.com> <mailto:nd@arm.com <mailto:nd@arm.com>>>
Subject: Re: [PATCH v2 2/4] StandaloneMmPkg/Hob: Integer Overflow in CreateHob()




Hi Gua,




Thank you for this patch.
Please see my response inline marked [SAMI].




Regards,




Sami Mujawar




On 11/01/2024, 09:15, "gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>>>" <gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com> <mailto:gua.guo@intel.com <mailto:gua.guo@intel.com>>>> wrote:








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








REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166 <https://bugzilla.tianocore.org/show_bug.cgi?id=4166> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166&gt;> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166&gt;> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166&gt;> <https://bugzilla.tianocore.org/show_bug.cgi?id=4166&amp;gt;&gt;>








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








diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
index 1550e1babc..bb8426dc0a 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCor
+++ eHobLib.c
@@ -34,6 +34,13 @@ CreateHob (
















HandOffHob = GetHobList ();
























+ //








+ // Check Length to avoid data overflow.








+ //








+ if (HobLength > MAX_UINT16 - 0x7) {








+ return NULL;
[SAMI] Although this fix is correct, I think it shifts the problem somewhere else. 
If the above condition occurs, a NULL is returned. A quick scan reveals that the calling functions do not check the returned value before use.
e.g. https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170 <https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170> <https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170> <https://github.com/tianocore/edk2/blob/master/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c#L167-L170&gt;>
There are multiple such places where the calling functions do not check the pointer returned by CreateHob(). 
I believe a similar situation can happen for the other patches in this series.
[/SAMI]




+ }








+








HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
























FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom;








--
2.39.2.windows.1































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