[edk2-devel] [PATCH v2 5/6] ArmPkg: MmCommunicationDxe: Update MM communicate `CommSize` check

[Kun Qin]

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

Current MM communicate routine from ArmPkg would conduct few checks prior
to proceeding with SMC calls. However, the inspection step is different
from PI specification.

This patch updated MM communicate input argument inspection routine to
assure `CommSize` represents "the size of the data buffer being passed
in" instead of the size of the data being used from data buffer, as
described by section `EFI_MM_COMMUNICATION2_PROTOCOL.Communicate()` in PI
specification.

Cc: Leif Lindholm <leif at nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>
Cc: Michael Kubacki <michael.kubacki at microsoft.com>

Signed-off-by: Kun Qin <kuqin12 at gmail.com>
---

Notes:
    v2:
    - Splitting patch into 3 of 4 [Ard]

 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 0283be430dff..2f89b7c5b6c4 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -44,13 +44,18 @@ STATIC EFI_HANDLE  mMmCommunicateHandle;
   @param[in] This                     The EFI_MM_COMMUNICATION_PROTOCOL instance.
   @param[in, out] CommBufferPhysical  Physical address of the MM communication buffer
   @param[in, out] CommBufferVirtual   Virtual address of the MM communication buffer
-  @param[in, out] CommSize            The size of the data buffer being passed in. On exit, the
-                                      size of data being returned. Zero if the handler does not
+  @param[in, out] CommSize            The size of the data buffer being passed in. On input,
+                                      when not omitted, the buffer should cover EFI_MM_COMMUNICATE_HEADER
+                                      and the value of MessageLength field. On exit, the size
+                                      of data being returned. Zero if the handler does not
                                       wish to reply with any data. This parameter is optional
                                       and may be NULL.
 
   @retval EFI_SUCCESS            The message was successfully posted.
-  @retval EFI_INVALID_PARAMETER  CommBufferPhysical was NULL or CommBufferVirtual was NULL.
+  @retval EFI_INVALID_PARAMETER  CommBufferPhysical or CommBufferVirtual was NULL, or
+                                 integer value pointed by CommSize does not cover
+                                 EFI_MM_COMMUNICATE_HEADER and the value of MessageLength
+                                 field.
   @retval EFI_BAD_BUFFER_SIZE    The buffer is too large for the MM implementation.
                                  If this error is returned, the MessageLength field
                                  in the CommBuffer header or the integer pointed by
@@ -96,8 +101,8 @@ MmCommunication2Communicate (
                sizeof (CommunicateHeader->HeaderGuid) +
                sizeof (CommunicateHeader->MessageLength);
 
-  // If the length of the CommBuffer is 0 then return the expected length.
-  if (CommSize != 0) {
+  // If CommSize is not omitted, perform size inspection before proceeding.
+  if (CommSize != NULL) {
     // This case can be used by the consumer of this driver to find out the
     // max size that can be used for allocating CommBuffer.
     if ((*CommSize == 0) ||
@@ -108,9 +113,9 @@ MmCommunication2Communicate (
     }
 
     //
-    // CommSize must match MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
+    // CommSize should cover at least MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
     //
-    if (*CommSize != BufferSize) {
+    if (*CommSize < BufferSize) {
       return EFI_INVALID_PARAMETER;
     }
   }
-- 
2.32.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85121): https://edk2.groups.io/g/devel/message/85121
Mute This Topic: https://groups.io/mt/87869777/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-