From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-57.freemail.mail.aliyun.com (out30-57.freemail.mail.aliyun.com [115.124.30.57]) by mx.groups.io with SMTP id smtpd.web08.7056.1634288820132675548 for ; Fri, 15 Oct 2021 02:07:00 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: linux.alibaba.com, ip: 115.124.30.57, mailfrom: huangming@linux.alibaba.com) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R611e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04357;MF=huangming@linux.alibaba.com;NM=1;PH=DS;RN=7;SR=0;TI=SMTPD_---0Us71SiA_1634288787; Received: from 842effaa37a8.tbsite.net(mailfrom:huangming@linux.alibaba.com fp:SMTPD_---0Us71SiA_1634288787) by smtp.aliyun-inc.com(127.0.0.1); Fri, 15 Oct 2021 17:06:57 +0800 From: "Ming Huang" To: devel@edk2.groups.io, sami.mujawar@arm.com, ardb+tianocore@kernel.org, jiewen.yao@intel.com, supreeth.venkatesh@arm.com Cc: ming.huang-@outlook.com, Ming Huang Subject: [PATCH edk2 v1 3/3] StandaloneMmPkg: Fix check buffer address failed issue from TF-A Date: Fri, 15 Oct 2021 17:06:23 +0800 Message-Id: <20211015090623.52511-4-huangming@linux.alibaba.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20211015090623.52511-1-huangming@linux.alibaba.com> References: <20211015090623.52511-1-huangming@linux.alibaba.com> There are two scene communicate with StandaloneMm(MM): 1 edk2 -> TF-A -> MM, communicate MM use non-secure buffer which specify by EFI_SECURE_PARTITION_BOOT_INFO.SpNsCommBufBase; 2 RAS scene: fiq -> TF-A -> MM, use secure buffer which specify by EFI_SECURE_PARTITION_BOOT_INFO.SpShareBufBase; For now, the second scene will failed because check buffer address. This patch add CheckBufferAddr() to support check address for secure buffer. Signed-off-by: Ming Huang --- StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c | 70 ++++++++++++++++---- StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 21 ++++++ StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h | 1 + 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c index 5dfaf9d751..63fab1bd78 100644 --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c @@ -50,6 +50,7 @@ EFI_MM_COMMUNICATE_HEADER **PerCpuGuidedEventContext = NULL; // Descriptor with whereabouts of memory used for communication with the normal world EFI_MMRAM_DESCRIPTOR mNsCommBuffer; +EFI_MMRAM_DESCRIPTOR mSCommBuffer; MP_INFORMATION_HOB_DATA *mMpInformationHobData; @@ -60,6 +61,58 @@ EFI_MM_CONFIGURATION_PROTOCOL mMmConfig = { STATIC EFI_MM_ENTRY_POINT mMmEntryPoint = NULL; +STATIC +EFI_STATUS +CheckBufferAddr ( + IN UINTN CommBufferAddr + ) +{ + UINTN CommBufferSize; + EFI_STATUS Status; + + Status = EFI_SUCCESS; + if (CommBufferAddr < mNsCommBuffer.PhysicalStart) { + Status = EFI_ACCESS_DENIED; + } + + if ((CommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >= + (mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize)) { + Status = EFI_INVALID_PARAMETER; + } + + // Find out the size of the buffer passed + CommBufferSize = ((EFI_MM_COMMUNICATE_HEADER *) CommBufferAddr)->MessageLength + + sizeof (EFI_MM_COMMUNICATE_HEADER); + + // perform bounds check. + if (CommBufferAddr + CommBufferSize >= + mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize) { + Status = EFI_ACCESS_DENIED; + } + + if (!EFI_ERROR (Status)) { + return EFI_SUCCESS; + } + + Status = EFI_SUCCESS; + if (CommBufferAddr < mSCommBuffer.PhysicalStart) { + Status = EFI_ACCESS_DENIED; + } + + if ((CommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >= + (mSCommBuffer.PhysicalStart + mSCommBuffer.PhysicalSize)) { + Status = EFI_INVALID_PARAMETER; + } + + // perform bounds check. + if (CommBufferAddr + CommBufferSize >= + mSCommBuffer.PhysicalStart + mSCommBuffer.PhysicalSize) { + Status = EFI_ACCESS_DENIED; + } + + return Status; +} + /** The PI Standalone MM entry point for the TF-A CPU driver. @@ -104,25 +157,16 @@ PiMmStandaloneArmTfCpuDriverEntry ( return EFI_INVALID_PARAMETER; } - if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) { - return EFI_ACCESS_DENIED; - } - - if ((NsCommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >= - (mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize)) { - return EFI_INVALID_PARAMETER; + Status = CheckBufferAddr (NsCommBufferAddr); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Check Buffer failed: %r\n", Status)); + return Status; } // Find out the size of the buffer passed NsCommBufferSize = ((EFI_MM_COMMUNICATE_HEADER *) NsCommBufferAddr)->MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER); - // perform bounds check. - if (NsCommBufferAddr + NsCommBufferSize >= - mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize) { - return EFI_ACCESS_DENIED; - } - GuidedEventContext = NULL; // Now that the secure world can see the normal world buffer, allocate // memory to copy the communication buffer to the secure world. diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c index fd9c59b4da..96dad20dd1 100644 --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c @@ -107,6 +107,7 @@ StandaloneMmCpuInitialize ( UINTN Index; UINTN ArraySize; VOID *HobStart; + EFI_MMRAM_HOB_DESCRIPTOR_BLOCK *MmramRangesHob; ASSERT (SystemTable != NULL); mMmst = SystemTable; @@ -186,6 +187,26 @@ StandaloneMmCpuInitialize ( CopyMem (&mNsCommBuffer, NsCommBufMmramRange, sizeof(EFI_MMRAM_DESCRIPTOR)); DEBUG ((DEBUG_INFO, "mNsCommBuffer: 0x%016lx - 0x%lx\n", mNsCommBuffer.CpuStart, mNsCommBuffer.PhysicalSize)); + Status = GetGuidedHobData ( + HobStart, + &gEfiMmPeiMmramMemoryReserveGuid, + (VOID **) &MmramRangesHob + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "MmramRangesHob data extraction failed - 0x%x\n", Status)); + return Status; + } + + // + // As CreateHobListFromBootInfo(), the base and size of buffer shared with + // privileged Secure world software is in second one. + // + CopyMem ( + &mSCommBuffer, + &MmramRangesHob->Descriptor[0] + 1, + sizeof(EFI_MMRAM_DESCRIPTOR) + ); + // // Extract the MP information from the Hoblist // diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h index 2c96439c15..2e03b20d85 100644 --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h @@ -30,6 +30,7 @@ extern EFI_MM_CPU_PROTOCOL mMmCpuState; // extern EFI_MM_COMMUNICATE_HEADER **PerCpuGuidedEventContext; extern EFI_MMRAM_DESCRIPTOR mNsCommBuffer; +extern EFI_MMRAM_DESCRIPTOR mSCommBuffer; extern MP_INFORMATION_HOB_DATA *mMpInformationHobData; extern EFI_MM_CONFIGURATION_PROTOCOL mMmConfig; -- 2.17.1