1. Patchew
  2. EDK2
  3. [edk2-devel] [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements
  4. View patch

[edk2-devel] [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue

Michael Kubacki posted 5 patches 4 years, 6 months ago
There is a newer version of this series
[edk2-devel] [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
Posted by Michael Kubacki 4 years, 6 months ago 
From: Michael Kubacki <michael.kubacki@microsoft.com>

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

The MessageLength field of EFI_MM_COMMUNICATE_HEADER as defined in
MdePkg/Include/Protocol/MmCommunication.h was updated to a fixed
size as opposed to UINTN to avoid ambiguity between different
caller enviornments.

This change updates the MessageLength usage in MinPlatformPkg to
support the new field structure, in turn, fixing a build issue.

Original edk2 change:
  https://bugzilla.tianocore.org/show_bug.cgi?id=3398

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c |  4 ++--
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c         | 15 ++++++++++++++-
 Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c                      | 10 +++++-----
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf       |  1 +
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c
index 3ceeb821fb95..80e8d26f4e1d 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c
@@ -106,7 +106,7 @@ GetSmiHandlerProfileDatabase(
   CommGetInfo->Header.ReturnStatus = (UINT64)-1;
   CommGetInfo->DataSize = 0;
 
-  CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+  CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
   Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize);
   if (EFI_ERROR(Status)) {
     DEBUG ((DEBUG_INFO, "SmiHandlerProfile: SmmCommunication - %r\n", Status));
@@ -139,7 +139,7 @@ GetSmiHandlerProfileDatabase(
   CommGetData->Header.DataLength = sizeof(*CommGetData);
   CommGetData->Header.ReturnStatus = (UINT64)-1;
 
-  CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+  CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
   Buffer = (UINT8 *)CommHeader + CommSize;
   Size -= CommSize;
 
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c
index c012e0afcbaa..e5efbd059954 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c
@@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Library/UefiLib.h>
 #include <Library/BaseMemoryLib.h>
+#include <Library/SafeIntLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <IndustryStandard/Acpi.h>
 #include <IndustryStandard/DmaRemappingReportingTable.h>
@@ -520,6 +521,7 @@ TestPointDxeSmmReadyToBootSmmPageProtection (
   UINTN                                               MemoryAttributesTableSize;
   EFI_STATUS                                          Status;
   UINTN                                               CommSize;
+  UINT64                                              LongCommSize;
   UINT8                                               *CommBuffer;
   EFI_SMM_COMMUNICATE_HEADER                          *CommHeader;
   EFI_SMM_COMMUNICATION_PROTOCOL                      *SmmCommunication;
@@ -620,7 +622,18 @@ TestPointDxeSmmReadyToBootSmmPageProtection (
     (UINTN)CommData->UefiMemoryAttributeTableSize
     );
 
-  CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + CommHeader->MessageLength;
+  Status = SafeUint64Add (OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), CommHeader->MessageLength, &LongCommSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: LongCommSize calculation - %r\n", Status));
+    return EFI_SUCCESS;
+  }
+
+  Status = SafeUint64ToUintn (LongCommSize, &CommSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: CommSize conversion - %r\n", Status));
+    return EFI_SUCCESS;
+  }
+
   Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize);
   if (EFI_ERROR(Status)) {
     DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: SmmCommunication - %r\n", Status));
diff --git a/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c b/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
index 3cc5ccfef6f4..8416b36f56ae 100644
--- a/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
+++ b/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
@@ -122,7 +122,7 @@ GetTestPointDataSmm (
   CommGetInfo->Header.ReturnStatus = (UINT64)-1;
   CommGetInfo->DataSize = 0;
 
-  CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+  CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
   Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize);
   if (EFI_ERROR(Status)) {
     DEBUG ((DEBUG_INFO, "SmiHandlerTestPoint: SmmCommunication - %r\n", Status));
@@ -155,7 +155,7 @@ GetTestPointDataSmm (
   CommGetData->Header.DataLength = sizeof(*CommGetData);
   CommGetData->Header.ReturnStatus = (UINT64)-1;
 
-  CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+  CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
   Buffer = (UINT8 *)CommHeader + CommSize;
   Size -= CommSize;
 
@@ -233,7 +233,7 @@ PublishSmmTestPoint (
   TestPoint = mSmmTestPointDatabase;
   while ((UINTN)TestPoint < (UINTN)mSmmTestPointDatabase + mSmmTestPointDatabaseSize) {
     TestPointSize = GetTestPointInfoSize (TestPoint, (UINTN)mSmmTestPointDatabase + mSmmTestPointDatabaseSize - (UINTN)TestPoint);
-    
+
     TestPointLibSetTable (TestPoint, TestPointSize);
 
     TestPoint = (ADAPTER_INFO_PLATFORM_TEST_POINT *)((UINTN)TestPoint + TestPointSize);
@@ -286,7 +286,7 @@ OnReadyToBoot (
   EFI_EVENT                         ReadyToBootLaterEvent;
 
   gBS->CloseEvent (Event);
-  
+
   Status = gBS->CreateEvent (
                   EVT_NOTIFY_SIGNAL,
                   TPL_CALLBACK,
@@ -295,7 +295,7 @@ OnReadyToBoot (
                   &ReadyToBootLaterEvent
                   );
   ASSERT_EFI_ERROR (Status);
-  
+
   gBS->SignalEvent (ReadyToBootLaterEvent);
 }
 
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
index 2ae1db4ee483..54b4015d6767 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
@@ -32,6 +32,7 @@ [LibraryClasses]
   TestPointLib
   PciSegmentLib
   PciSegmentInfoLib
+  SafeIntLib
 
 [Packages]
   MinPlatformPkg/MinPlatformPkg.dec
-- 
2.28.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78711): https://edk2.groups.io/g/devel/message/78711
Mute This Topic: https://groups.io/mt/84686303/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
Posted by Nate DeSimone 4 years, 6 months ago 
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Thursday, August 5, 2021 7:57 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2-platforms][PATCH v1 1/5]
> MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3531
> 
> The MessageLength field of EFI_MM_COMMUNICATE_HEADER as defined in
> MdePkg/Include/Protocol/MmCommunication.h was updated to a fixed size
> as opposed to UINTN to avoid ambiguity between different caller
> enviornments.
> 
> This change updates the MessageLength usage in MinPlatformPkg to support
> the new field structure, in turn, fixing a build issue.
> 
> Original edk2 change:
>   https://bugzilla.tianocore.org/show_bug.cgi?id=3398
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
> miHandlerInstrument.c |  4 ++--
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.c         | 15 ++++++++++++++-
>  Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
> | 10 +++++-----
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.inf       |  1 +
>  4 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmiHandlerInstrument.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmiHandlerInstrument.c
> index 3ceeb821fb95..80e8d26f4e1d 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmiHandlerInstrument.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckSmiHandlerInstrument.c
> @@ -106,7 +106,7 @@ GetSmiHandlerProfileDatabase(
>    CommGetInfo->Header.ReturnStatus = (UINT64)-1;
>    CommGetInfo->DataSize = 0;
> 
> -  CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
> >MessageLength;
> +  CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> + (UINTN)CommHeader->MessageLength;
>    Status = SmmCommunication->Communicate(SmmCommunication,
> CommBuffer, &CommSize);
>    if (EFI_ERROR(Status)) {
>      DEBUG ((DEBUG_INFO, "SmiHandlerProfile: SmmCommunication - %r\n",
> Status)); @@ -139,7 +139,7 @@ GetSmiHandlerProfileDatabase(
>    CommGetData->Header.DataLength = sizeof(*CommGetData);
>    CommGetData->Header.ReturnStatus = (UINT64)-1;
> 
> -  CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
> >MessageLength;
> +  CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> + (UINTN)CommHeader->MessageLength;
>    Buffer = (UINT8 *)CommHeader + CommSize;
>    Size -= CommSize;
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.c
> index c012e0afcbaa..e5efbd059954 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTe
> +++ stPointCheckLib.c
> @@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include
> <Library/DebugLib.h>  #include <Library/UefiLib.h>  #include
> <Library/BaseMemoryLib.h>
> +#include <Library/SafeIntLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <IndustryStandard/Acpi.h>
>  #include <IndustryStandard/DmaRemappingReportingTable.h>
> @@ -520,6 +521,7 @@ TestPointDxeSmmReadyToBootSmmPageProtection (
>    UINTN                                               MemoryAttributesTableSize;
>    EFI_STATUS                                          Status;
>    UINTN                                               CommSize;
> +  UINT64                                              LongCommSize;
>    UINT8                                               *CommBuffer;
>    EFI_SMM_COMMUNICATE_HEADER                          *CommHeader;
>    EFI_SMM_COMMUNICATION_PROTOCOL
> *SmmCommunication;
> @@ -620,7 +622,18 @@ TestPointDxeSmmReadyToBootSmmPageProtection
> (
>      (UINTN)CommData->UefiMemoryAttributeTableSize
>      );
> 
> -  CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
> CommHeader->MessageLength;
> +  Status = SafeUint64Add (OFFSET_OF
> (EFI_SMM_COMMUNICATE_HEADER, Data),
> + CommHeader->MessageLength, &LongCommSize);  if (EFI_ERROR (Status))
> {
> +    DEBUG ((DEBUG_INFO,
> "TestPointDxeSmmReadyToBootSmmPageProtection: LongCommSize
> calculation - %r\n", Status));
> +    return EFI_SUCCESS;
> +  }
> +
> +  Status = SafeUint64ToUintn (LongCommSize, &CommSize);  if (EFI_ERROR
> + (Status)) {
> +    DEBUG ((DEBUG_INFO,
> "TestPointDxeSmmReadyToBootSmmPageProtection: CommSize conversion
> - %r\n", Status));
> +    return EFI_SUCCESS;
> +  }
> +
>    Status = SmmCommunication->Communicate(SmmCommunication,
> CommBuffer, &CommSize);
>    if (EFI_ERROR(Status)) {
>      DEBUG ((DEBUG_INFO,
> "TestPointDxeSmmReadyToBootSmmPageProtection: SmmCommunication -
> %r\n", Status)); diff --git
> a/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe
> .c
> b/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe
> .c
> index 3cc5ccfef6f4..8416b36f56ae 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe
> .c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubD
> +++ xe.c
> @@ -122,7 +122,7 @@ GetTestPointDataSmm (
>    CommGetInfo->Header.ReturnStatus = (UINT64)-1;
>    CommGetInfo->DataSize = 0;
> 
> -  CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
> >MessageLength;
> +  CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> + (UINTN)CommHeader->MessageLength;
>    Status = SmmCommunication->Communicate(SmmCommunication,
> CommBuffer, &CommSize);
>    if (EFI_ERROR(Status)) {
>      DEBUG ((DEBUG_INFO, "SmiHandlerTestPoint: SmmCommunication -
> %r\n", Status)); @@ -155,7 +155,7 @@ GetTestPointDataSmm (
>    CommGetData->Header.DataLength = sizeof(*CommGetData);
>    CommGetData->Header.ReturnStatus = (UINT64)-1;
> 
> -  CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
> >MessageLength;
> +  CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
> + (UINTN)CommHeader->MessageLength;
>    Buffer = (UINT8 *)CommHeader + CommSize;
>    Size -= CommSize;
> 
> @@ -233,7 +233,7 @@ PublishSmmTestPoint (
>    TestPoint = mSmmTestPointDatabase;
>    while ((UINTN)TestPoint < (UINTN)mSmmTestPointDatabase +
> mSmmTestPointDatabaseSize) {
>      TestPointSize = GetTestPointInfoSize (TestPoint,
> (UINTN)mSmmTestPointDatabase + mSmmTestPointDatabaseSize -
> (UINTN)TestPoint);
> -
> +
>      TestPointLibSetTable (TestPoint, TestPointSize);
> 
>      TestPoint = (ADAPTER_INFO_PLATFORM_TEST_POINT
> *)((UINTN)TestPoint + TestPointSize); @@ -286,7 +286,7 @@
> OnReadyToBoot (
>    EFI_EVENT                         ReadyToBootLaterEvent;
> 
>    gBS->CloseEvent (Event);
> -
> +
>    Status = gBS->CreateEvent (
>                    EVT_NOTIFY_SIGNAL,
>                    TPL_CALLBACK,
> @@ -295,7 +295,7 @@ OnReadyToBoot (
>                    &ReadyToBootLaterEvent
>                    );
>    ASSERT_EFI_ERROR (Status);
> -
> +
>    gBS->SignalEvent (ReadyToBootLaterEvent);  }
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.inf
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.inf
> index 2ae1db4ee483..54b4015d6767 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.inf
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTe
> +++ stPointCheckLib.inf
> @@ -32,6 +32,7 @@ [LibraryClasses]
>    TestPointLib
>    PciSegmentLib
>    PciSegmentInfoLib
> +  SafeIntLib
> 
>  [Packages]
>    MinPlatformPkg/MinPlatformPkg.dec
> --
> 2.28.0.windows.1



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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.