From: "Ming Huang" <huangming@linux.alibaba.com>
To: devel@edk2.groups.io, sami.mujawar@arm.com,
"ardb+tianocore@kernel.org" <ardb+tianocore@kernel.org>,
"jiewen.yao@intel.com" <jiewen.yao@intel.com>,
Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>
Cc: "ming.huang-@outlook.com" <ming.huang-@outlook.com>, nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH edk2 v3 3/3] StandaloneMmPkg: Fix check buffer address failed issue from TF-A
Date: Fri, 13 May 2022 17:21:24 +0800 [thread overview]
Message-ID: <89f516c9-fa8d-3f33-d7e1-4c3799ffaebb@linux.alibaba.com> (raw)
In-Reply-To: <843FF6EA-3E67-4802-B562-83FB7ADA823E@arm.com>
Hi Sami,
Sorry for the late reply.
https://github.com/waip23/edk2/tree/upstread-fix-mm-issue
branch: upstread-fix-mm-issue
Regards,
Ming Huang
在 4/6/22 5:03 PM, Sami Mujawar 写道:
> Hi Ming,
>
> I am not sure if this is an issue at my end, but I cannot apply this patch series, can you check, please?
> Also, is it possible to share these patches on a Github branch.
>
> Regards,
>
> Sami Mujawar
>
> On 30/03/2022, 10:37, "Ming Huang" <huangming@linux.alibaba.com> wrote:
>
> Hi,
>
> Any comment about this series?
>
> 在 12/31/21 7:06 PM, Ming Huang 写道:
> > 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 <huangming@linux.alibaba.com>
> > ---
> > StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c | 54 +++++++++++++++-----
> > StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 21 ++++++++
> > StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h | 1 +
> > 3 files changed, 63 insertions(+), 13 deletions(-)
> >
> > diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
> > index 5dfaf9d751..d0ba415671 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,42 @@ EFI_MM_CONFIGURATION_PROTOCOL mMmConfig = {
> >
> > STATIC EFI_MM_ENTRY_POINT mMmEntryPoint = NULL;
> >
> > +STATIC
> > +EFI_STATUS
> > +CheckBufferAddr (
> > + IN UINTN BufferAddr
> > + )
> > +{
> > + UINT64 NsCommBufferEnd;
> > + UINT64 SCommBufferEnd;
> > + UINT64 CommBufferEnd;
> > +
> > + NsCommBufferEnd = mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize;
> > + SCommBufferEnd = mSCommBuffer.PhysicalStart + mSCommBuffer.PhysicalSize;
> > +
> > + if ((BufferAddr >= mNsCommBuffer.PhysicalStart) &&
> > + (BufferAddr < NsCommBufferEnd)) {
> > + CommBufferEnd = NsCommBufferEnd;
> > + } else if ((BufferAddr >= mSCommBuffer.PhysicalStart) &&
> > + (BufferAddr < SCommBufferEnd)) {
> > + CommBufferEnd = SCommBufferEnd;
> > + } else {
> > + return EFI_ACCESS_DENIED;
> > + }
> > +
> > + if ((CommBufferEnd - BufferAddr) < sizeof (EFI_MM_COMMUNICATE_HEADER)) {
> > + return EFI_ACCESS_DENIED;
> > + }
> > +
> > + // perform bounds check.
> > + if ((CommBufferEnd - BufferAddr - sizeof (EFI_MM_COMMUNICATE_HEADER)) <
> > + ((EFI_MM_COMMUNICATE_HEADER *) BufferAddr)->MessageLength) {
> > + return EFI_ACCESS_DENIED;
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > /**
> > The PI Standalone MM entry point for the TF-A CPU driver.
> >
> > @@ -104,25 +141,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;
> >
>
>
>
>
>
>
next prev parent reply other threads:[~2022-05-13 9:21 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-31 11:06 [PATCH edk2 v3 0/3] Fix several issues in StanaloneMmPkg Ming Huang
2021-12-31 11:06 ` [PATCH edk2 v3 1/3] StandaloneMmPkg: Fix issue about SpPcpuSharedBufSize field Ming Huang
2021-12-31 11:06 ` [PATCH edk2 v3 2/3] StandaloneMmPkg: Replace DEBUG_INFO with DEBUG_ERROR Ming Huang
2021-12-31 11:06 ` [PATCH edk2 v3 3/3] StandaloneMmPkg: Fix check buffer address failed issue from TF-A Ming Huang
2022-03-30 9:37 ` Ming Huang
2022-04-06 9:03 ` Sami Mujawar
2022-04-11 1:58 ` [edk2-devel] " Ming Huang
2022-05-13 9:21 ` Ming Huang [this message]
2022-07-07 11:07 ` Sami Mujawar
2022-01-22 8:56 ` [PATCH edk2 v3 0/3] Fix several issues in StanaloneMmPkg Ming Huang
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=89f516c9-fa8d-3f33-d7e1-4c3799ffaebb@linux.alibaba.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