From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web09.12856.1603799270114495673 for ; Tue, 27 Oct 2020 04:47:50 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=JeXOOzpU; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603799269; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WTGIYqQJcAtEGy8CryQoMXqNv2qSp5b9J0yCLWOdyTY=; b=JeXOOzpU24F0glZr8/QndH0eLOl9S32/Nc9Xm61s8rY8ovg8M23pxO+GfCNb/+JdHjSOAl 2pDE6hWA3GHHe7qup5Co1Wkygsm9gVDJcdgm6SqTd7yNO9IpKKDoBeZ5I8It94vdrHKyPb heGJLVwUUSo7toP1y1/yogqFFYPNI4k= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-590-WYtBYO_7Nr-yg89iyrij6w-1; Tue, 27 Oct 2020 07:47:45 -0400 X-MC-Unique: WYtBYO_7Nr-yg89iyrij6w-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 05842809DD9; Tue, 27 Oct 2020 11:47:43 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-132.ams2.redhat.com [10.36.114.132]) by smtp.corp.redhat.com (Postfix) with ESMTP id D7C2D10013C4; Tue, 27 Oct 2020 11:47:41 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Gcd: Check memory allocation when initializing memory To: devel@edk2.groups.io, jbrasen@nvidia.com Cc: dandan.bi@intel.com, gaoliming@byosoft.com.cn References: <20201023221007.1097763-1-jbrasen@nvidia.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 27 Oct 2020 12:47:40 +0100 MIME-Version: 1.0 In-Reply-To: <20201023221007.1097763-1-jbrasen@nvidia.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Jeff, On 10/24/20 00:10, Jeff Brasen wrote: > CoreInitializeMemoryServices was not checking for any existing memory > allocation created in the HOB producer phase. If there are memory > allocations outside of the region covered by the HOB List then Gcd could > select that region for memory which can result in the memory allocation > to not be handled and memory overwrites. > > Signed-off-by: Jeff Brasen > --- > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 60 +++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) can you please run "BaseTools/Scripts/SetupGit.py" in your edk2 clone? Thanks, Laszlo > > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > index 2d8c076f71..4a22ee96b7 100644 > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > @@ -2097,6 +2097,62 @@ CalculateTotalMemoryBinSizeNeeded ( > return TotalSize; > > } > > > > +/** > > + Find the largest region in the specified region that is not covered by an existing memory allocation > > + > > + @param BaseAddress On input start of the region to check. > > + On output start of the largest free region. > > + @param Length On input size of region to check. > > + On output size of the largest free region. > > + @param MemoryHob Hob pointer for the first memory allocation pointer to check > > +**/ > > +VOID > > +FindLargestFreeRegion ( > > + IN OUT EFI_PHYSICAL_ADDRESS *BaseAddress, > > + IN OUT UINT64 *Length, > > + IN EFI_HOB_MEMORY_ALLOCATION *MemoryHob > > + ) > > +{ > > + EFI_PHYSICAL_ADDRESS TopAddress; > > + > > + TopAddress = *BaseAddress + *Length; > > + while (MemoryHob != NULL) { > > + EFI_PHYSICAL_ADDRESS AllocatedTop; > > + > > + AllocatedTop = MemoryHob->AllocDescriptor.MemoryBaseAddress + MemoryHob->AllocDescriptor.MemoryLength; > > + > > + if ((MemoryHob->AllocDescriptor.MemoryBaseAddress >= *BaseAddress) && > > + (AllocatedTop <= TopAddress)) { > > + EFI_PHYSICAL_ADDRESS LowerBase; > > + UINT64 LowerSize; > > + EFI_PHYSICAL_ADDRESS UpperBase; > > + UINT64 UpperSize; > > + > > + LowerBase = *BaseAddress; > > + LowerSize = MemoryHob->AllocDescriptor.MemoryBaseAddress - *BaseAddress; > > + UpperBase = AllocatedTop; > > + UpperSize = TopAddress - AllocatedTop; > > + > > + if (LowerSize != 0) { > > + FindLargestFreeRegion (&LowerBase, &LowerSize, (EFI_HOB_MEMORY_ALLOCATION *) GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob))); > > + } > > + if (UpperSize != 0) { > > + FindLargestFreeRegion (&UpperBase, &UpperSize, (EFI_HOB_MEMORY_ALLOCATION *) GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob))); > > + } > > + > > + if (UpperSize >= LowerSize) { > > + *Length = UpperSize; > > + *BaseAddress = UpperBase; > > + } else { > > + *Length = LowerSize; > > + *BaseAddress = LowerBase; > > + } > > + return; > > + } > > + MemoryHob = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob)); > > + } > > +} > > + > > /** > > External function. Initializes memory services based on the memory > > descriptor HOBs. This function is responsible for priming the memory > > @@ -2235,6 +2291,7 @@ CoreInitializeMemoryServices ( > Attributes = PhitResourceHob->ResourceAttribute; > > BaseAddress = PageAlignAddress (PhitHob->EfiMemoryTop); > > Length = PageAlignLength (ResourceHob->PhysicalStart + ResourceHob->ResourceLength - BaseAddress); > > + FindLargestFreeRegion (&BaseAddress, &Length, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION)); > > if (Length < MinimalMemorySizeNeeded) { > > // > > // If that range is not large enough to intialize the DXE Core, then > > @@ -2242,6 +2299,7 @@ CoreInitializeMemoryServices ( > // > > BaseAddress = PageAlignAddress (PhitHob->EfiFreeMemoryBottom); > > Length = PageAlignLength (PhitHob->EfiFreeMemoryTop - BaseAddress); > > + //This region is required to have no memory allocation inside it, skip check for entries in HOB List > > if (Length < MinimalMemorySizeNeeded) { > > // > > // If that range is not large enough to intialize the DXE Core, then > > @@ -2249,6 +2307,7 @@ CoreInitializeMemoryServices ( > // > > BaseAddress = PageAlignAddress (ResourceHob->PhysicalStart); > > Length = PageAlignLength ((UINT64)((UINTN)*HobStart - BaseAddress)); > > + FindLargestFreeRegion (&BaseAddress, &Length, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION)); > > } > > } > > break; > > @@ -2312,6 +2371,7 @@ CoreInitializeMemoryServices ( > // > > TestedMemoryBaseAddress = PageAlignAddress (ResourceHob->PhysicalStart); > > TestedMemoryLength = PageAlignLength (ResourceHob->PhysicalStart + ResourceHob->ResourceLength - TestedMemoryBaseAddress); > > + FindLargestFreeRegion (&TestedMemoryBaseAddress, &TestedMemoryLength, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION)); > > if (TestedMemoryLength < MinimalMemorySizeNeeded) { > > continue; > > } >