public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kun Qin" <kuqin12@gmail.com>
To: devel@edk2.groups.io
Cc: Leif Lindholm <leif@nuviainc.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Bret Barkelew <Bret.Barkelew@microsoft.com>,
	Michael Kubacki <michael.kubacki@microsoft.com>,
	Sami Mujawar <sami.mujawar@arm.com>
Subject: [PATCH v3 6/6] ArmPkg: MmCommunicationDxe: Update MM communicate `MessageLength` check
Date: Tue, 25 Jan 2022 11:39:09 -0800	[thread overview]
Message-ID: <20220125193909.491-7-kuqin12@gmail.com> (raw)
In-Reply-To: <20220125193909.491-1-kuqin12@gmail.com>

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 that "if the `MessageLength` is zero, or too large for the MM
implementation to manage, the MM implementation must update the
`MessageLength` to reflect the size of the `Data` buffer that it can
tolerate", as described by `EFI_MM_COMMUNICATION_PROTOCOL.Communicate()`
section in PI specification.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
    - Splitting patch into 4 of 4 [Ard]
    - Uncrustify style update
    
    v3:
    - Added cc entry and reviewed-by tag [Sami]

 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 2f89b7c5b6c4..85d9034555f0 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -92,6 +92,7 @@ MmCommunication2Communicate (
     return EFI_INVALID_PARAMETER;
   }
 
+  Status            = EFI_SUCCESS;
   CommunicateHeader = CommBufferVirtual;
   // CommBuffer is a mandatory parameter. Hence, Rely on
   // MessageLength + Header to ascertain the
@@ -109,28 +110,33 @@ MmCommunication2Communicate (
         (*CommSize > mNsCommBuffMemRegion.Length))
     {
       *CommSize = mNsCommBuffMemRegion.Length;
-      return EFI_BAD_BUFFER_SIZE;
+      Status    = EFI_BAD_BUFFER_SIZE;
     }
 
     //
     // CommSize should cover at least MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
     //
     if (*CommSize < BufferSize) {
-      return EFI_INVALID_PARAMETER;
+      Status = EFI_INVALID_PARAMETER;
     }
   }
 
   //
-  // If the buffer size is 0 or greater than what can be tolerated by the MM
+  // If the message length is 0 or greater than what can be tolerated by the MM
   // environment then return the expected size.
   //
-  if ((BufferSize == 0) ||
+  if ((CommunicateHeader->MessageLength == 0) ||
       (BufferSize > mNsCommBuffMemRegion.Length))
   {
     CommunicateHeader->MessageLength = mNsCommBuffMemRegion.Length -
                                        sizeof (CommunicateHeader->HeaderGuid) -
                                        sizeof (CommunicateHeader->MessageLength);
-    return EFI_BAD_BUFFER_SIZE;
+    Status = EFI_BAD_BUFFER_SIZE;
+  }
+
+  // MessageLength or CommSize check has failed, return here.
+  if (EFI_ERROR (Status)) {
+    return Status;
   }
 
   // SMC Function ID
-- 
2.34.1.windows.1


  parent reply	other threads:[~2022-01-25 19:39 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-25 19:39 [PATCH v3 0/6] MM communicate functionality in variable policy Kun Qin
2022-01-25 19:39 ` [PATCH v3 1/6] MdeModulePkg: VariableSmmRuntimeDxe: Fix Variable Policy Message Length Kun Qin
2022-01-25 19:39 ` [PATCH v3 2/6] MdePkg: MmCommunication2: Update MM communicate2 function description Kun Qin
2022-01-25 19:39 ` [PATCH v3 3/6] ArmPkg: MmCommunicationDxe: MM communicate function argument attributes Kun Qin
2022-01-25 19:39 ` [PATCH v3 4/6] ArmPkg: MmCommunicationDxe: Update MM communicate `CommBuffer**` checks Kun Qin
2022-01-25 19:39 ` [PATCH v3 5/6] ArmPkg: MmCommunicationDxe: Update MM communicate `CommSize` check Kun Qin
2022-01-25 19:39 ` Kun Qin [this message]
2022-01-26  1:41 ` 回复: [PATCH v3 0/6] MM communicate functionality in variable policy gaoliming
2022-01-27 23:05   ` [edk2-devel] " Kun Qin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220125193909.491-7-kuqin12@gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox