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>
Subject: [PATCH v2 5/6] ArmPkg: MmCommunicationDxe: Update MM communicate `CommSize` check
Date: Mon, 20 Dec 2021 17:33:33 -0800 [thread overview]
Message-ID: <20211221013334.1751-6-kuqin12@gmail.com> (raw)
In-Reply-To: <20211221013334.1751-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 `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@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@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
next prev parent reply other threads:[~2021-12-21 1:34 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-21 1:33 [PATCH v2 0/6] MM communicate functionality in variable policy Kun Qin
2021-12-21 1:33 ` [PATCH v2 1/6] MdeModulePkg: VariableSmmRuntimeDxe: Fix Variable Policy Message Length Kun Qin
2022-01-11 1:18 ` 回复: [edk2-devel] " gaoliming
2021-12-21 1:33 ` [PATCH v2 2/6] MdePkg: MmCommunication2: Update MM communicate2 function description Kun Qin
2022-01-04 3:22 ` 回复: [edk2-devel] " gaoliming
2021-12-21 1:33 ` [PATCH v2 3/6] ArmPkg: MmCommunicationDxe: MM communicate function argument attributes Kun Qin
2021-12-21 1:33 ` [PATCH v2 4/6] ArmPkg: MmCommunicationDxe: Update MM communicate `CommBuffer**` checks Kun Qin
2021-12-21 1:33 ` Kun Qin [this message]
2021-12-21 1:33 ` [PATCH v2 6/6] ArmPkg: MmCommunicationDxe: Update MM communicate `MessageLength` check Kun Qin
2022-01-18 18:40 ` [edk2-devel] [PATCH v2 0/6] MM communicate functionality in variable policy Kun Qin
2022-01-18 22:04 ` Rebecca Cran
2022-01-18 22:06 ` Kun Qin
2022-01-19 7:59 ` Sami Mujawar
2022-01-19 9:37 ` Sami Mujawar
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=20211221013334.1751-6-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