From nobody Sat May 4 01:36:14 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+78880+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+78880+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=posteo.de ARC-Seal: i=1; a=rsa-sha256; t=1628451635; cv=none; d=zohomail.com; s=zohoarc; b=jfikaH1kri+X1Cslk5qLHix0wrFfomut6RNIvGrjxvcanercFl/0ZK9Fy3V8wU1Nm5LNqWer83XV3gX1pqvD4p1Sg9WNA2/wH+wJmWlpOUZnb2Y7DJfgryV817O1OWB4Tv8zcg/z97AXirueGlRCna5JTXgGyJmAnL51cLga6gc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1628451635; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=p9vNnKZHkbdAE8d8hM7ZPf72o5J0IEEdeI+EhUyb594=; b=abZyrluZ05nR8X3Vk3a2Bf5AIZS7TkoV9iHpizROASySYbvR2KDs7QmS6v81x1RAg/ZOMT20xO+d8Q85n/gXI2Zk7srjetG5sPwMDm/ZEtF3kYWaQ3/xWtG6kV7pqUnAcuh9OUruTHBzxCLdREv7UlYu+T//5W+lleIVdePXQsA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+78880+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1628451635740538.824591364664; Sun, 8 Aug 2021 12:40:35 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id t00HYY1788612xy4SZRIH2ZA; Sun, 08 Aug 2021 12:40:35 -0700 X-Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web09.13695.1628451633175724287 for ; Sun, 08 Aug 2021 12:40:33 -0700 X-Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id A3EE8240027 for ; Sun, 8 Aug 2021 21:40:31 +0200 (CEST) X-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4GjV1W0PRdz6tmF; Sun, 8 Aug 2021 21:40:31 +0200 (CEST) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= To: devel@edk2.groups.io Cc: Jian J Wang , Hao A Wu , Dandan Bi , Liming Gao , Vitaly Cheptsov Subject: [edk2-devel] [PATCH] MdeModulePkg/CoreDxe: Drop caller-allocated image buffers Date: Sun, 8 Aug 2021 19:39:39 +0000 Message-Id: <376e8f0af0ecac7debed5975f0741d880382c866.1628364368.git.mhaeuser@posteo.de> In-Reply-To: <5df11a13422732b9c03c120775a2b4dd0a49182f.1628444003.git.mhaeuser@posteo.de> References: <5df11a13422732b9c03c120775a2b4dd0a49182f.1628444003.git.mhaeuser@posteo.de> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mhaeuser@posteo.de X-Gm-Message-State: P3H6as8gGxQUtBIN4NgY4a54x1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1628451635; bh=mdhHd/tIEdphyepuuI6Nhrzb8CL6InZnBbQkomDUBJ8=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=fYgMIjTeshf9uxVH9TYt1IXi3VYDQrCxhUKpwqCtdBv2d3GmLWPoocUbTcdA8vwqIL4 hATiWxmqdEEqcu/BFsICWlkPKTd8L6fRFP2BoMp7yUSk+6f4e6uipyjz13vOYrtnwdJiC Lu2c7rCMjh35XwPTNJARmBunLbqIM5kn1K8= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1628451637390100016 Content-Type: text/plain; charset="utf-8" The current image loading code supports using an caller-allocated buffer to load an UEFI image into. This concept is inherently flawed as a caller would need to parse the image itself first to retrieve the appropriate destination size. As the only caller does not use this functionality, remove it. Further drop the EntryPoint parameter, as it is unused as well. Cc: Jian J Wang Cc: Hao A Wu Cc: Dandan Bi Cc: Liming Gao Cc: Vitaly Cheptsov Signed-off-by: Marvin H=C3=A4user --- MdeModulePkg/Core/Dxe/Image/Image.c | 199 ++++++-------------- 1 file changed, 62 insertions(+), 137 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Im= age/Image.c index 641a5715b112..1de83f96e5ed 100644 --- a/MdeModulePkg/Core/Dxe/Image/Image.c +++ b/MdeModulePkg/Core/Dxe/Image/Image.c @@ -540,8 +540,6 @@ CoreIsImageTypeSupported ( boot selection. @param Pe32Handle The handle of PE32 image @param Image PE image to be loaded - @param DstBuffer The buffer to store the image - @param EntryPoint A pointer to the entry point @param Attribute The bit mask of attributes to set for th= e load PE image =20 @@ -557,13 +555,10 @@ CoreLoadPeImage ( IN BOOLEAN BootPolicy, IN VOID *Pe32Handle, IN LOADED_IMAGE_PRIVATE_DATA *Image, - IN EFI_PHYSICAL_ADDRESS DstBuffer OPTIONAL, - OUT EFI_PHYSICAL_ADDRESS *EntryPoint OPTIONAL, IN UINT32 Attribute ) { EFI_STATUS Status; - BOOLEAN DstBufAlocated; UINTN Size; =20 ZeroMem (&Image->ImageContext, sizeof (Image->ImageContext)); @@ -615,93 +610,63 @@ CoreLoadPeImage ( // // Allocate memory of the correct memory type aligned on the required im= age boundary // - DstBufAlocated =3D FALSE; - if (DstBuffer =3D=3D 0) { - // - // Allocate Destination Buffer as caller did not pass it in - // =20 - if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) { - Size =3D (UINTN)Image->ImageContext.ImageSize + Image->ImageContext.= SectionAlignment; - } else { - Size =3D (UINTN)Image->ImageContext.ImageSize; - } - - Image->NumberOfPages =3D EFI_SIZE_TO_PAGES (Size); - - // - // If the image relocations have not been stripped, then load at any a= ddress. - // Otherwise load at the address at which it was linked. - // - // Memory below 1MB should be treated reserved for CSM and there shoul= d be - // no modules whose preferred load addresses are below 1MB. - // - Status =3D EFI_OUT_OF_RESOURCES; - // - // If Loading Module At Fixed Address feature is enabled, the module s= hould be loaded to - // a specified address. - // - if (PcdGet64(PcdLoadModuleAtFixAddressEnable) !=3D 0 ) { - Status =3D GetPeCoffImageFixLoadingAssignedAddress (&(Image->ImageCo= ntext)); - - if (EFI_ERROR (Status)) { - // - // If the code memory is not ready, invoke CoreAllocatePage with= AllocateAnyPages to load the driver. - // - DEBUG ((EFI_D_INFO|EFI_D_LOAD, "LOADING MODULE FIXED ERROR: Load= ing module at fixed address failed since specified memory is not available.= \n")); - - Status =3D CoreAllocatePages ( - AllocateAnyPages, - (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemor= yType), - Image->NumberOfPages, - &Image->ImageContext.ImageAddress - ); - } - } else { - if (Image->ImageContext.ImageAddress >=3D 0x100000 || Image->ImageCo= ntext.RelocationsStripped) { - Status =3D CoreAllocatePages ( - AllocateAddress, - (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryT= ype), - Image->NumberOfPages, - &Image->ImageContext.ImageAddress - ); - } - if (EFI_ERROR (Status) && !Image->ImageContext.RelocationsStripped) { - Status =3D CoreAllocatePages ( - AllocateAnyPages, - (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryT= ype), - Image->NumberOfPages, - &Image->ImageContext.ImageAddress - ); - } - } - if (EFI_ERROR (Status)) { - return Status; - } - DstBufAlocated =3D TRUE; + if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) { + Size =3D (UINTN)Image->ImageContext.ImageSize + Image->ImageContext.Se= ctionAlignment; } else { - // - // Caller provided the destination buffer - // - - if (Image->ImageContext.RelocationsStripped && (Image->ImageContext.Im= ageAddress !=3D DstBuffer)) { + Size =3D (UINTN)Image->ImageContext.ImageSize; + } + + Image->NumberOfPages =3D EFI_SIZE_TO_PAGES (Size); + + // + // If the image relocations have not been stripped, then load at any add= ress. + // Otherwise load at the address at which it was linked. + // + // Memory below 1MB should be treated reserved for CSM and there should = be + // no modules whose preferred load addresses are below 1MB. + // + Status =3D EFI_OUT_OF_RESOURCES; + // + // If Loading Module At Fixed Address feature is enabled, the module sho= uld be loaded to + // a specified address. + // + if (PcdGet64(PcdLoadModuleAtFixAddressEnable) !=3D 0 ) { + Status =3D GetPeCoffImageFixLoadingAssignedAddress (&(Image->ImageCont= ext)); + + if (EFI_ERROR (Status)) { // - // If the image relocations were stripped, and the caller provided a - // destination buffer address that does not match the address that t= he - // image is linked at, then the image cannot be loaded. + // If the code memory is not ready, invoke CoreAllocatePage with All= ocateAnyPages to load the driver. // - return EFI_INVALID_PARAMETER; + DEBUG ((EFI_D_INFO|EFI_D_LOAD, "LOADING MODULE FIXED ERROR: Loading = module at fixed address failed since specified memory is not available.\n")= ); + + Status =3D CoreAllocatePages ( + AllocateAnyPages, + (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryTyp= e), + Image->NumberOfPages, + &Image->ImageContext.ImageAddress + ); } - - if (Image->NumberOfPages !=3D 0 && - Image->NumberOfPages < - (EFI_SIZE_TO_PAGES ((UINTN)Image->ImageContext.ImageSize + Image->= ImageContext.SectionAlignment))) { - Image->NumberOfPages =3D EFI_SIZE_TO_PAGES ((UINTN)Image->ImageConte= xt.ImageSize + Image->ImageContext.SectionAlignment); - return EFI_BUFFER_TOO_SMALL; + } else { + if (Image->ImageContext.ImageAddress >=3D 0x100000 || Image->ImageCont= ext.RelocationsStripped) { + Status =3D CoreAllocatePages ( + AllocateAddress, + (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryTy= pe), + Image->NumberOfPages, + &Image->ImageContext.ImageAddress + ); } - - Image->NumberOfPages =3D EFI_SIZE_TO_PAGES ((UINTN)Image->ImageContext= .ImageSize + Image->ImageContext.SectionAlignment); - Image->ImageContext.ImageAddress =3D DstBuffer; + if (EFI_ERROR (Status) && !Image->ImageContext.RelocationsStripped) { + Status =3D CoreAllocatePages ( + AllocateAnyPages, + (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryTy= pe), + Image->NumberOfPages, + &Image->ImageContext.ImageAddress + ); + } + } + if (EFI_ERROR (Status)) { + return Status; } =20 Image->ImageBasePage =3D Image->ImageContext.ImageAddress; @@ -783,13 +748,6 @@ CoreLoadPeImage ( } } =20 - // - // Fill in the entry point of the image if it is available - // - if (EntryPoint !=3D NULL) { - *EntryPoint =3D Image->ImageContext.EntryPoint; - } - // // Print the load address and the PDB file name if it is available // @@ -854,11 +812,9 @@ Done: // Free memory. // =20 - if (DstBufAlocated) { - CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages); - Image->ImageContext.ImageAddress =3D 0; - Image->ImageBasePage =3D 0; - } + CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages); + Image->ImageContext.ImageAddress =3D 0; + Image->ImageBasePage =3D 0; =20 if (Image->ImageContext.FixupData !=3D NULL) { CoreFreePool (Image->ImageContext.FixupData); @@ -906,13 +862,11 @@ CoreLoadedImageInfo ( Unloads EFI image from memory. =20 @param Image EFI image - @param FreePage Free allocated pages =20 **/ VOID CoreUnloadAndCloseImage ( - IN LOADED_IMAGE_PRIVATE_DATA *Image, - IN BOOLEAN FreePage + IN LOADED_IMAGE_PRIVATE_DATA *Image ) { EFI_STATUS Status; @@ -1038,7 +992,7 @@ CoreUnloadAndCloseImage ( // // Free the Image from memory // - if ((Image->ImageBasePage !=3D 0) && FreePage) { + if (Image->ImageBasePage !=3D 0) { CoreFreePages (Image->ImageBasePage, Image->NumberOfPages); } =20 @@ -1074,15 +1028,8 @@ CoreUnloadAndCloseImage ( @param SourceBuffer If not NULL, a pointer to the memory loc= ation containing a copy of the image to be loa= ded. @param SourceSize The size in bytes of SourceBuffer. - @param DstBuffer The buffer to store the image - @param NumberOfPages If not NULL, it inputs a pointer to the = page - number of DstBuffer and outputs a pointe= r to - the page number of the image. If this nu= mber is - not enough, return EFI_BUFFER_TOO_SMALL= and - this parameter contains the required num= ber. @param ImageHandle Pointer to the returned image handle tha= t is created when the image is successfully l= oaded. - @param EntryPoint A pointer to the entry point @param Attribute The bit mask of attributes to set for th= e load PE image =20 @@ -1112,10 +1059,7 @@ CoreLoadImageCommon ( IN EFI_DEVICE_PATH_PROTOCOL *FilePath, IN VOID *SourceBuffer OPTIONAL, IN UINTN SourceSize, - IN EFI_PHYSICAL_ADDRESS DstBuffer OPTIONAL, - IN OUT UINTN *NumberOfPages OPTIONAL, OUT EFI_HANDLE *ImageHandle, - OUT EFI_PHYSICAL_ADDRESS *EntryPoint OPTIONAL, IN UINT32 Attribute ) { @@ -1320,13 +1264,6 @@ CoreLoadImageCommon ( Image->Info.FilePath =3D DuplicateDevicePath (FilePath); Image->Info.ParentHandle =3D ParentImageHandle; =20 - - if (NumberOfPages !=3D NULL) { - Image->NumberOfPages =3D *NumberOfPages ; - } else { - Image->NumberOfPages =3D 0 ; - } - // // Install the protocol interfaces for this image // don't fire notifications yet @@ -1343,22 +1280,13 @@ CoreLoadImageCommon ( } =20 // - // Load the image. If EntryPoint is Null, it will not be set. + // Load the image. // - Status =3D CoreLoadPeImage (BootPolicy, &FHand, Image, DstBuffer, EntryP= oint, Attribute); + Status =3D CoreLoadPeImage (BootPolicy, &FHand, Image, Attribute); if (EFI_ERROR (Status)) { - if ((Status =3D=3D EFI_BUFFER_TOO_SMALL) || (Status =3D=3D EFI_OUT_OF_= RESOURCES)) { - if (NumberOfPages !=3D NULL) { - *NumberOfPages =3D Image->NumberOfPages; - } - } goto Done; } =20 - if (NumberOfPages !=3D NULL) { - *NumberOfPages =3D Image->NumberOfPages; - } - // // Register the image in the Debug Image Info Table if the attribute is = set // @@ -1438,7 +1366,7 @@ Done: // if (EFI_ERROR (Status)) { if (Image !=3D NULL) { - CoreUnloadAndCloseImage (Image, (BOOLEAN)(DstBuffer =3D=3D 0)); + CoreUnloadAndCloseImage (Image); Image =3D NULL; } } else if (EFI_ERROR (SecurityStatus)) { @@ -1514,10 +1442,7 @@ CoreLoadImage ( FilePath, SourceBuffer, SourceSize, - (EFI_PHYSICAL_ADDRESS) (UINTN) NULL, - NULL, ImageHandle, - NULL, EFI_LOAD_PE_IMAGE_ATTRIBUTE_RUNTIME_REGISTRATION | EFI_LOAD_P= E_IMAGE_ATTRIBUTE_DEBUG_IMAGE_INFO_TABLE_REGISTRATION ); =20 @@ -1734,7 +1659,7 @@ CoreStartImage ( // unload it // if (EFI_ERROR (Image->Status) || Image->Type =3D=3D EFI_IMAGE_SUBSYSTEM_= EFI_APPLICATION) { - CoreUnloadAndCloseImage (Image, TRUE); + CoreUnloadAndCloseImage (Image); // // ImageHandle may be invalid after the image is unloaded, so use NULL= handle to record perf log. // @@ -1799,7 +1724,7 @@ CoreExit ( // // The image has not been started so just free its resources // - CoreUnloadAndCloseImage (Image, TRUE); + CoreUnloadAndCloseImage (Image); Status =3D EFI_SUCCESS; goto Done; } @@ -1901,7 +1826,7 @@ CoreUnloadImage ( // // if the Image was not started or Unloaded O.K. then clean up // - CoreUnloadAndCloseImage (Image, TRUE); + CoreUnloadAndCloseImage (Image); } =20 Done: --=20 2.31.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#78880): https://edk2.groups.io/g/devel/message/78880 Mute This Topic: https://groups.io/mt/84754052/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-