From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f169.google.com (mail-pg1-f169.google.com [209.85.215.169]) by mx.groups.io with SMTP id smtpd.web11.69481.1638232773553688713 for ; Mon, 29 Nov 2021 16:39:33 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=e1yRKzyz; spf=pass (domain: gmail.com, ip: 209.85.215.169, mailfrom: kuqin12@gmail.com) Received: by mail-pg1-f169.google.com with SMTP id f125so4602461pgc.0 for ; Mon, 29 Nov 2021 16:39:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ZDQKz4tbDP9539enxYVXsdoUa6t9H+rlVmbIqzlVBI8=; b=e1yRKzyz6GORcOyJWL2jSE7y603ExLYRqWIia6J5zqhtD/bvHf3yoNJbS2jMgqVUQe DKWG2yIgRP9+TshZEobO0oxkBo/qgXwIDBH5HLckG/kR4WQkB3HGbStITMHnDV49fRkj 0FiaDVqZfYncm4lzIDaMUHmeoMvp94GAn6yUBuf0xECzPN9NCNjpDmH8/WopysRxMM/L 3SkP5PDXONHZm8u1qYg6QS1aWdiUyM313gPkMby+ZsvT2+Q9Iw6codow6vqIgmtFeBVo lKDhYSm66if1pH8xaIN9DRcK1rlCDAa0qKcXCx/ee7y7oqhaI7Rf/YNfizyRILiWlCzU 2LAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ZDQKz4tbDP9539enxYVXsdoUa6t9H+rlVmbIqzlVBI8=; b=hutmfNw/oGOoO/cR6pk7HHr7pRWSgANrPmD7ezVgqHKaxc53ePaRcd6fM2HC4o3YdV 4+zDNVIbp6/7fQY7yQqVxvKGKbopb9bG5gXM//DE2rMPl6Th/tRRsFyUJxN+3BsqLixe RBYEixvdC09V5l2E1srU0Ii08BsBcf9/jN6qLDh3YgtPP7IluTPPLUsLw78QXYXUattj VOXMCiJuFg1QtrPcvqtNA3iE/KZKxxGObAWib18+yILd4aPIJrvUtEzdpF6Rz/htmr+6 wB5ZxgGaqn6gaqmCRv6Fi9QCUwaezvSWYN8OJjji4mNp9UnGz1JGDESN6g1MPNCdmNI1 /3Cg== X-Gm-Message-State: AOAM5302nwdkA6gUkFc6oPXSXmQEscSMst0VobVO93lNA31GT6lv/ni/ oK7SQox0o164okqjIVZ/qBL6nIr18D1vUQ== X-Google-Smtp-Source: ABdhPJy9HB2CSWfVKGot2TNTXmz2Xnmi1mQxjHepFnrynKyTEY14rTPdbdhxCfyjThxwhVHJ1lvGxA== X-Received: by 2002:a63:b51:: with SMTP id a17mr37111233pgl.283.1638232772998; Mon, 29 Nov 2021 16:39:32 -0800 (PST) Return-Path: Received: from localhost.localdomain ([50.35.70.63]) by smtp.gmail.com with ESMTPSA id v10sm19286119pfu.123.2021.11.29.16.39.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 29 Nov 2021 16:39:32 -0800 (PST) From: "Kun Qin" To: devel@edk2.groups.io Cc: Leif Lindholm , Ard Biesheuvel , Bret Barkelew , Michael Kubacki Subject: [PATCH v1 2/2] ArmPkg: MmCommunicationDxe: Update MM communicate input arguments checks Date: Mon, 29 Nov 2021 16:39:02 -0800 Message-Id: <20211130003902.1884-3-kuqin12@gmail.com> X-Mailer: git-send-email 2.32.0.windows.1 In-Reply-To: <20211130003902.1884-1-kuqin12@gmail.com> References: <20211130003902.1884-1-kuqin12@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751 Current MM communicate routine from ArmPkg would conduct few steps before proceeding with SMC calls. However, some inspection steps are different from PI specification. This patch updated MM communicate input argument inspection routine to match the following PI descriptions: 1. Return code `EFI_INVALID_PARAMETER` represents "the `CommBuffer**` parameters do not refer to the same location in memory". 2. `CommSize` represents "the size of the data buffer being passed in" instead of "the size of the data being used from data buffer". 3. Regarding `MessageLength`, "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". Cc: Leif Lindholm Cc: Ard Biesheuvel Cc: Bret Barkelew Cc: Michael Kubacki Signed-off-by: Kun Qin --- ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 44 ++++++++++++-------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c index b1e309580988..8a2bd222957f 100644 --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c @@ -41,15 +41,19 @@ STATIC EFI_HANDLE mMmCommunicateHandle; This function provides a service to send and receive messages from a registered UEFI service. - @param[in] This The EFI_MM_COMMUNICATION_PROTOCOL instance. - @param[in] CommBufferPhysical Physical address of the MM communication buffer - @param[in] CommBufferVirtual Virtual address of the MM communication buffer - @param[in] CommSize The size of the data buffer being passed in. 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. + @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 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 @@ -82,10 +86,11 @@ MmCommunication2Communicate ( // // Check parameters // - if (CommBufferVirtual == NULL) { + if (CommBufferVirtual == NULL || CommBufferPhysical == NULL) { return EFI_INVALID_PARAMETER; } + Status = EFI_SUCCESS; CommunicateHeader = CommBufferVirtual; // CommBuffer is a mandatory parameter. Hence, Rely on // MessageLength + Header to ascertain the @@ -95,33 +100,38 @@ 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) || (*CommSize > mNsCommBuffMemRegion.Length)) { *CommSize = mNsCommBuffMemRegion.Length; - return EFI_BAD_BUFFER_SIZE; + Status = EFI_BAD_BUFFER_SIZE; } // - // CommSize must match MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER); + // CommSize should cover at least MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER); // - if (*CommSize != BufferSize) { - return EFI_INVALID_PARAMETER; + if (*CommSize < BufferSize) { + 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.32.0.windows.1