From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR02-HE1-obe.outbound.protection.outlook.com (EUR02-HE1-obe.outbound.protection.outlook.com [40.107.1.57]) by mx.groups.io with SMTP id smtpd.web12.17478.1639429436866525611 for ; Mon, 13 Dec 2021 13:03:58 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=ky1D9xfA; spf=pass (domain: arm.com, ip: 40.107.1.57, mailfrom: sami.mujawar@arm.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Uwb3yJ0hGIvCEPaRImIb+8SYKNrrdEnff1SYUQeCM6w=; b=ky1D9xfA5rrGb+BiaYjxEPozwH8eNlWieCQrjXWRk+pDR4xGvRN3Rp5mhuaaiLYp7+X3sHxig03olJFcjYgOlHbNHaNx/W2XrXFoMw8Etgh3ss2WA9+BV+YqZ9ylJwlQIhR/FDoIhJq77KPx9C8R3JlByK663LK/M1Y57LAXEPM= Received: from AM8P189CA0021.EURP189.PROD.OUTLOOK.COM (2603:10a6:20b:218::26) by DBAPR08MB5605.eurprd08.prod.outlook.com (2603:10a6:10:1af::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4778.14; Mon, 13 Dec 2021 21:03:53 +0000 Received: from VE1EUR03FT023.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:218:cafe::b7) by AM8P189CA0021.outlook.office365.com (2603:10a6:20b:218::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4778.16 via Frontend Transport; Mon, 13 Dec 2021 21:03:53 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by VE1EUR03FT023.mail.protection.outlook.com (10.152.18.133) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4778.12 via Frontend Transport; Mon, 13 Dec 2021 21:03:52 +0000 Received: ("Tessian outbound 1cd1a01725a6:v110"); Mon, 13 Dec 2021 21:03:52 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: ffd09eca2dd0c009 X-CR-MTA-TID: 64aa7808 Received: from 0fbf61dd0aa1.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 6270B34D-058C-45D9-9E7F-E0040DE611EA.1; Mon, 13 Dec 2021 21:03:41 +0000 Received: from EUR04-DB3-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 0fbf61dd0aa1.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Mon, 13 Dec 2021 21:03:41 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=m2mt/3PpAj8yCWiPM1v+qF6H/DF+LHyGE/f97UIosKLuNYvuS3FlIByWg0WBEtS2g2CXcRzlh6HxkU1j1eJEJU3/QEM9cJCa26vo1oyIXv53bXqSveKvM4HgYNw4VYqHULKdzzHWy3cjdyIDr+DR4qvgOvPbx4+dxcvp7I6eRTN4qbxuAbnW3geHUVosCdxZr03k4CtprSLL79FcUg9EhPczedT9T000fYkojmPaiqpE3+PCx4yyPne0fzFtK0emSGmBw2yZsYP+7rq8Ok+tXSqD4o4vmKKPtNpeBwYuwHl4kXL51kuvMrEN8Kpeetw/7++Homis67XH+1FFOscRVg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Uwb3yJ0hGIvCEPaRImIb+8SYKNrrdEnff1SYUQeCM6w=; b=IDvP+n0UVZ/NKmrbcrPCJE9H5G5rBFZjHYHlbMqvdUpo3Mss1WdPUuFN+5D7IB+beM205V1TfEriSI9Y+86vUUY8QhYmaS9ixWY2M9Z2aGk9OORyAk3bm5HX6055wiAOKGVlZMmMoOF7do36rpvCh9XKJi4CSocrhroKwk21Vl60xhGuhgUlxzx41cT4ckJYN0rp6bi+nEyJr4YYtucirnrZSQt6Xx1u7uRCOwXBfA7yZzaS+p26r6Z+C+mWRD07LruUMJGA9zwiiIqim7mXFU8qN5N6tKxdU6XNRdPR3QGWIQ13K2/Eyy3mWnaFm3XHMNdlftWq7kNecBmoH/duWA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Uwb3yJ0hGIvCEPaRImIb+8SYKNrrdEnff1SYUQeCM6w=; b=ky1D9xfA5rrGb+BiaYjxEPozwH8eNlWieCQrjXWRk+pDR4xGvRN3Rp5mhuaaiLYp7+X3sHxig03olJFcjYgOlHbNHaNx/W2XrXFoMw8Etgh3ss2WA9+BV+YqZ9ylJwlQIhR/FDoIhJq77KPx9C8R3JlByK663LK/M1Y57LAXEPM= Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by AM5PR0801MB2033.eurprd08.prod.outlook.com (2603:10a6:203:4c::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4778.11; Mon, 13 Dec 2021 21:03:37 +0000 Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::bdcf:cfa6:b2bb:38ac]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::bdcf:cfa6:b2bb:38ac%6]) with mapi id 15.20.4755.021; Mon, 13 Dec 2021 21:03:37 +0000 Subject: Re: [edk2-devel] [PATCH v1 2/2] ArmPkg: MmCommunicationDxe: Update MM communicate input arguments checks To: devel@edk2.groups.io, kuqin12@gmail.com Cc: Leif Lindholm , Ard Biesheuvel , Bret Barkelew , Michael Kubacki , nd References: <20211130003902.1884-1-kuqin12@gmail.com> <20211130003902.1884-3-kuqin12@gmail.com> From: "Sami Mujawar" Message-ID: <486bca9b-c3ed-265c-76a6-5f8392f554c1@arm.com> Date: Mon, 13 Dec 2021 21:03:42 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.0.1 In-Reply-To: <20211130003902.1884-3-kuqin12@gmail.com> X-ClientProxiedBy: LO4P265CA0032.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2ae::18) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 Received: from [10.1.196.43] (217.140.106.50) by LO4P265CA0032.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2ae::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4755.21 via Frontend Transport; Mon, 13 Dec 2021 21:03:37 +0000 X-MS-Office365-Filtering-Correlation-Id: db89e5a8-fab4-4256-076e-08d9be7c1181 X-MS-TrafficTypeDiagnostic: AM5PR0801MB2033:EE_|VE1EUR03FT023:EE_|DBAPR08MB5605:EE_ X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:8882;OLM:8882; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: esD6HeXvyh3VRjUqdP9hwqvJ9WDF/miBESxIIwfQ/tNF1gs4+dgyyQn5B8wjEsWDxmaUAC0DrMNugQdRvj6diy+jOWKNA4ytjG8tPfezCHu9k+ZUwb0iQI4no4ue0W954pQclmACM8PSUayw90fJPLcPpHHpa/ELRhv+7RfuvNsEyC9fqSXcdAHthwanExEzAbz1rkWyImBOTZZfm/9OA52vCxYLlzcBq5EocjnM2jmaCuMdE2bMuCUYW7ff2o0gF+wGtQc3GL5XwrKEj0lz3OFUaiPtx12/VXCqg4QC4ccpa+7NTpP3bhvLVmR/IzJh6i3X5BPX1Sau1aG5zxHAnhOdIs4uX7g9KM5CUkd/lTImP2Xug5yiyE9BQ+XSnLUaWXn5ScJsz4vBVCk3OmN78fpkN15r3vVnJZDYXc162JZj23+WnvIj++pifcA7F6xpN/hRIobbG070q1TYCw1dWtc9nP+gXCgMnn4eCNlBj6ZHb3PrdZU2FeGMgO/B4KB99f03HSDDg6TMndrbHyljZ+g3mchDxfu1wi8bXV3NMhLbN83qBiKvxmdORpVGBdiiQy0c/QUjQp/oSjA5NoH6gGrC9kLTcrR5bNaPKJTuY7F0rZHstNZPXdz73NGj5w+0BtgAGXUiyGuGUIfBsSFWes9GxsmrRtxpMXf4dR1sqIw3g9K7EVtU3MzfIOaE1XFJazxOgWs3LqJ1tjRaN96dzM1WB31AtLR72DuHJZ1IPJM5h8IfOG4TjEMV9KJmlSebD210V7vpXgFIqU7DAdkFBCck79qaE9H9kcP/JsC4bXzl+xdUmIMJKEdF8ClIvNNVEm5ym5c2MfMH5P4HXDyXp8PV9VChYbNru37ti2sibS0= X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS8PR08MB6806.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(6029001)(4636009)(366004)(31686004)(83380400001)(956004)(6486002)(4326008)(44832011)(5660300002)(36756003)(2616005)(6666004)(86362001)(8936002)(54906003)(8676002)(38350700002)(316002)(2906002)(66946007)(31696002)(26005)(16576012)(508600001)(966005)(186003)(19627235002)(53546011)(38100700002)(15650500001)(66476007)(52116002)(66556008)(45080400002)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB2033 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Return-Path: Sami.Mujawar@arm.com X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT023.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 82d553d0-d4e0-4de0-f882-08d9be7c0859 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: ckWOUvoEdB/p5OHN5lCX8STXfr8MgDL6B8fV650dtlIvdiC5cMa8LB9i8LlV11uM8OXsmwWm2uaQbgtUlJU8iGK3R7vjH5KG4zOPn4gj+u0J0ca4huY7LNXzn2RENjv27tzoNNwl/Yehl4ENScu1dTdJ+KPo/xBIB6ODkR4d0XvrPNx9ZjzaXDOYbN7nIC3SgJV0dbN4suceTnHe6NEPQTjpIfTidy6p6I5jE3kiO8rfg4V1Y7V3l5p+KMAyvjxNfn9xAPrvE4rj9q2LMvI1lxdCcrR74VlzMMrE0n2UW6UjALCIyhGs2CqcWM5x6sK2FQnfdTIqvInU5/WAPF/RRPbaLG9AYaJA2FNImbU54tkBNZvFMYfWHmKDs9b0DsAgLLsB/9FTH0a1t39mCHkKMD4LCTIVwg7cu5D6tNPDXDKfSR2S0r52oMfTGbe+EHtmZ9jidMmvBy9+NeZfQ9g4k06FMN6O//VJyj+Ms/dVeRq4gepuQKaixga4KLPQ1/5MXgCICHwZ7jtWbYbZi0e5dQ679CzvkEUX8b9dEPY/AqICZXkbrpqxHB4XY761+mq0PCo6pTOnKY31+HanL18rd57Y/KoY5owaDTbhVxAsb30VusViWEd4fS1tC21cCcqL+OLNo/4Z8EX2sv5X4/QqrC0Go/vTUgevBbv1/yJCa8avA1j9e9ZdZr0O0ac2QsvPoksMHQ4//vdJONjOzOrHpZUrAuqIjHX4PCR8xmn7NWOsULaDdurDYXWDmtmnepdjglAuBh7qYkOiYJ/keZV1RbSAsJyDbi1wa7CWACSVxt36ywpWp1pJyLJsb3YGdUCa X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(6029001)(4636009)(36840700001)(46966006)(2616005)(8936002)(70586007)(31696002)(6486002)(19627235002)(956004)(86362001)(44832011)(47076005)(336012)(5660300002)(82310400004)(6666004)(45080400002)(8676002)(36860700001)(2906002)(508600001)(83380400001)(186003)(26005)(15650500001)(81166007)(356005)(70206006)(54906003)(53546011)(966005)(4326008)(36756003)(16576012)(316002)(31686004)(43740500002);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2021 21:03:52.7276 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: db89e5a8-fab4-4256-076e-08d9be7c1181 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: VE1EUR03FT023.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBAPR08MB5605 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB Hi Kun, Thank you for this patch. These changes look good to me. Reviewed-by: Sami Mujawar Regards, Sami Mujawar On 30/11/2021 12:39 AM, Kun Qin via groups.io wrote: > 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