From nobody Thu Nov 14 07:16:55 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+110658+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+110658+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=gmail.com ARC-Seal: i=1; a=rsa-sha256; t=1699031846; cv=none; d=zohomail.com; s=zohoarc; b=cuNXlu9Zcp+sXCQINvvxBHuM+XoAIhqiY1Axz8JBqZLTnSinscrT+n0+4Z8fl7RYW4c23lf0gU2iUnoy6oZ1UlBGp2SUzrahTdWYUN8XGGoMH8k5YSXjK+HogWdlIyr10ksHMRdb6uajWNC2Em4WiW2PI9fL8RsxwcLQfpzMGKY= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1699031846; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Reply-To:References:Sender:Subject:Subject:To:To:Message-Id; bh=Ht4FBttdXeyfu0CFy0HO8+gZYIyRFp54Nnno5BHcZoM=; b=P3XsAUoltnCXHLODolVZTWPPm0H4IS1+f0MJoBapuVIMgzAAGNRv9jUm70JzWvE5V+s3bGjuS0fTYS5yh43SNPzP7wZBEjBiby7MTZdoeixKjBvr8xse5v0zxqkrXXcrQ+zdYgQgWgEH+pDld88MQeDdDTH/U7gUq9x7y1WCS0k= 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+110658+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 1699031846503512.0858771360689; Fri, 3 Nov 2023 10:17:26 -0700 (PDT) Return-Path: DKIM-Signature: a=rsa-sha256; bh=E+ZP0GC/xqJHoYp1PHtG3s+UjvgFl7muukLfzYN0nVY=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1699031846; v=1; b=vqV7bImn4PfQ1qZsvVUQT0cx5aUMD/JccKAv0JMlK2fP2GSOfUqU+FINAB2V1SgRJb5dkmJc +EoaL8TCZT1AsZss5qZWshe6BcbpOHUhug4kF/4eYrlEOoEq50OuBQ1UnVUsxJVVFUQDN5cOXIW qyaay2zyMKJNKyCHGYX+HS7g= X-Received: by 127.0.0.2 with SMTP id RIwGYY1788612xejEbGj0DB2; Fri, 03 Nov 2023 10:17:26 -0700 X-Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by mx.groups.io with SMTP id smtpd.web10.59755.1699031844474691411 for ; Fri, 03 Nov 2023 10:17:24 -0700 X-Received: by mail-pl1-f170.google.com with SMTP id d9443c01a7336-1cc394f4cdfso18311325ad.0 for ; Fri, 03 Nov 2023 10:17:24 -0700 (PDT) X-Gm-Message-State: jenm3isXLZK6Z6Jvu1aKURORx1787277AA= X-Google-Smtp-Source: AGHT+IGeVZT/7tcUTDZEs5R7MEvb/nJGaoo610NwF505lReUbwiF7c33sLNpwMQjCF5nk+m31yemUg== X-Received: by 2002:a17:902:d2d2:b0:1cc:5ac3:8544 with SMTP id n18-20020a170902d2d200b001cc5ac38544mr14420557plc.28.1699031843631; Fri, 03 Nov 2023 10:17:23 -0700 (PDT) X-Received: from localhost.localdomain ([50.46.253.1]) by smtp.gmail.com with ESMTPSA id b8-20020a170903228800b001bf5e24b2a8sm1625152plh.174.2023.11.03.10.17.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Nov 2023 10:17:23 -0700 (PDT) From: "Taylor Beebe" To: devel@edk2.groups.io Cc: Jian J Wang , Liming Gao , Dandan Bi Subject: [edk2-devel] [PATCH v4 10/14] MdeModulePkg: Add NULL checks and Return Status to ImagePropertiesRecordLib Date: Fri, 3 Nov 2023 10:17:02 -0700 Message-ID: <20231103171706.148-11-taylor.d.beebe@gmail.com> In-Reply-To: <20231103171706.148-1-taylor.d.beebe@gmail.com> References: <20231103171706.148-1-taylor.d.beebe@gmail.com> MIME-Version: 1.0 Precedence: Bulk 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,taylor.d.beebe@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1699031848964100040 Content-Type: text/plain; charset="utf-8" Update function headers to clarify the contract of each function and improve readability. Add NULL checks to all functions that take a pointer as an argument. Add return status to functions that may need to return early due to invalid input. Cc: Jian J Wang Cc: Liming Gao Cc: Dandan Bi Signed-off-by: Taylor Beebe Reviewed-by: Liming Gao --- MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c |= 290 ++++++++++++-------- MdeModulePkg/Include/Library/ImagePropertiesRecordLib.h |= 137 ++++----- 2 files changed, 246 insertions(+), 181 deletions(-) diff --git a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesR= ecordLib.c b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesR= ecordLib.c index 379eb0c6cccd..c9378679e7bb 100644 --- a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLi= b.c +++ b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLi= b.c @@ -22,14 +22,13 @@ ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size))) =20 /** - Converts a number of EFI_PAGEs to a size in bytes. + Converts a number of pages to a size in bytes. =20 NOTE: Do not use EFI_PAGES_TO_SIZE because it handles UINTN only. =20 - @param Pages The number of EFI_PAGES. + @param[in] Pages The number of EFI_PAGES. =20 - @return The number of bytes associated with the number of EFI_PAGEs spe= cified - by Pages. + @retval The number of bytes associated with the input number of pages. **/ STATIC UINT64 @@ -45,10 +44,9 @@ EfiPagesToSize ( =20 NOTE: Do not use EFI_SIZE_TO_PAGES because it handles UINTN only. =20 - @param Size A size in bytes. + @param[in] Size A size in bytes. =20 - @return The number of EFI_PAGESs associated with the number of bytes sp= ecified - by Size. + @retval The number of pages associated with the input number of bytes. =20 **/ STATIC @@ -61,12 +59,12 @@ EfiSizeToPages ( } =20 /** - Sort memory map entries based upon PhysicalStart, from low to high. + Sort memory map entries based upon PhysicalStart from low to high. =20 - @param MemoryMap A pointer to the buffer in which firmware= places - the current memory map. - @param MemoryMapSize Size, in bytes, of the MemoryMap buffer. - @param DescriptorSize Size, in bytes, of an individual EFI_MEMO= RY_DESCRIPTOR. + @param[in, out] MemoryMap A pointer to the buffer in which firmwar= e places + the current memory map. + @param[in] MemoryMapSize Size, in bytes, of the MemoryMap buffer. + @param[in] DescriptorSize Size, in bytes, of an individual EFI_MEM= ORY_DESCRIPTOR. **/ STATIC VOID @@ -105,11 +103,12 @@ SortMemoryMap ( /** Return the first image record, whose [ImageBase, ImageSize] covered by [= Buffer, Length]. =20 - @param Buffer Start Address - @param Length Address length - @param ImageRecordList Image record list + @param[in] Buffer Starting Address + @param[in] Length Length to check + @param[in] ImageRecordList A list of IMAGE_PROPERTIES_RECORD entries to= check against + the memory range Buffer -> Buffer + Length =20 - @return first image record covered by [buffer, length] + @retval The first image record covered by [Buffer, Length] **/ STATIC IMAGE_PROPERTIES_RECORD * @@ -144,17 +143,19 @@ GetImageRecordByAddress ( } =20 /** - Set the memory map to new entries, according to one old entry, - based upon PE code section and data section in image record + Break up the input OldRecord into multiple new records based on the code + and data sections in the input ImageRecord. =20 - @param ImageRecord An image record whose [ImageBase, ImageSi= ze] covered - by old memory map entry. - @param NewRecord A pointer to several new memory map entri= es. - The caller gurantee the buffer size be 1 + - (SplitRecordCount * DescriptorSize) calcu= lated - below. - @param OldRecord A pointer to one old memory map entry. - @param DescriptorSize Size, in bytes, of an individual EFI_MEMO= RY_DESCRIPTOR. + @param[in] ImageRecord An IMAGE_PROPERTIES_RECORD whose Ima= geBase and + ImageSize is covered by by OldRecord. + @param[in, out] NewRecord A pointer to several new memory map = entries. + The caller gurantee the buffer size = be 1 + + (SplitRecordCount * DescriptorSize) = calculated + below. + @param[in] OldRecord A pointer to one old memory map entr= y. + @param[in] DescriptorSize The size, in bytes, of an individual= EFI_MEMORY_DESCRIPTOR. + + @retval The number of new descriptors created. **/ STATIC UINTN @@ -244,16 +245,16 @@ SetNewRecord ( } =20 /** - Return the max number of new splitted entries, according to one old entr= y, - based upon PE code section and data section. + Return the maximum number of new entries required to describe the code a= nd data sections + of all images covered by the input OldRecord. =20 - @param OldRecord A pointer to one old memory map entry. - @param ImageRecordList A list of IMAGE_PROPERTIES_RECORD entries= used when searching - for an image record contained by the memo= ry range described in - the existing EFI memory map descriptor Ol= dRecord + @param[in] OldRecord A pointer to one old memory map entry. + @param[in] ImageRecordList A list of IMAGE_PROPERTIES_RECORD entries = used when searching + for an image record contained by the memor= y range described by + OldRecord =20 - @retval 0 no entry need to be splitted. - @return the max number of new splitted entries + @retval The maximum number of new descriptors required to describe the = code and data sections + of all images covered by OldRecord. **/ STATIC UINTN @@ -289,22 +290,20 @@ GetMaxSplitRecordCount ( } =20 /** - Split the memory map to new entries, according to one old entry, - based upon PE code section and data section. + Split the memory map into new entries based upon the PE code and data se= ctions + in ImageRecordList covered by the input OldRecord. =20 - @param OldRecord A pointer to one old memory map entr= y. - @param NewRecord A pointer to several new memory map = entries. - The caller gurantee the buffer size = be 1 + - (SplitRecordCount * DescriptorSize) = calculated - below. - @param MaxSplitRecordCount The max number of splitted entries - @param DescriptorSize Size, in bytes, of an individual EFI= _MEMORY_DESCRIPTOR. - @param ImageRecordList A list of IMAGE_PROPERTIES_RECORD en= tries used when searching - for an image record contained by the= memory range described in - the existing EFI memory map descript= or OldRecord + @param[in] OldRecord A pointer to one old memory map = entry. + @param[in, out] NewRecord A pointer to several new memory = map entries. + The caller gurantee the buffer s= ize be + (SplitRecordCount * DescriptorSi= ze). + @param[in] MaxSplitRecordCount The maximum number of entries po= st-split. + @param[in] DescriptorSize The size, in bytes, of an indivi= dual EFI_MEMORY_DESCRIPTOR. + @param[in] ImageRecordList A list of IMAGE_PROPERTIES_RECOR= D entries used when searching + for an image record contained by= the memory range described in + the existing EFI memory map desc= riptor OldRecord =20 - @retval 0 no entry is splitted. - @return the real number of splitted record. + @retval The number of split entries. **/ STATIC UINTN @@ -402,56 +401,47 @@ SplitRecord ( } =20 /** - Split the original memory map, and add more entries to describe PE code = section and data section. - This function will set EfiRuntimeServicesData to be EFI_MEMORY_XP. - This function will merge entries with same attributes finally. + Split the original memory map and add more entries to describe PE code + and data sections for each image in the input ImageRecordList. =20 - NOTE: It assumes PE code/data section are page aligned. - NOTE: It assumes enough entry is prepared for new memory map. + NOTE: This function assumes PE code/data section are page aligned. + NOTE: This function assumes there are enough entries for the new memory = map. =20 - Split table: - +---------------+ - | Record X | - +---------------+ - | Record RtCode | - +---------------+ - | Record Y | - +---------------+ - =3D=3D> - +---------------+ - | Record X | - +---------------+ ---- - | Record RtData | | - +---------------+ | - | Record RtCode | |-> PE/COFF1 - +---------------+ | - | Record RtData | | - +---------------+ ---- - | Record RtData | | - +---------------+ | - | Record RtCode | |-> PE/COFF2 - +---------------+ | - | Record RtData | | - +---------------+ ---- - | Record Y | - +---------------+ + | | | | | | | | + | 4K PAGE | DATA | CODE | DATA | CODE | DATA | 4K PAGE | + | | | | | | | | + Assume the above memory region is the result of one split memory map des= criptor. It's unlikely + that a linker will orient an image this way, but the caller must assume = the worst case scenario. + This image layout example contains code sections oriented in a way that = maximizes the number of + descriptors which would be required to describe each section. To ensure = we have enough space + for every descriptor of the broken up memory map, the caller must assume= that every image will + have the maximum number of code sections oriented in a way which maximiz= es the number of data + sections with unrelated memory regions flanking each image within a sing= le descriptor. =20 - @param MemoryMapSize A pointer to the size, in bytes,= of the - MemoryMap buffer. On input, this= is the size of - old MemoryMap before split. The = actual buffer - size of MemoryMap is MemoryMapSi= ze + - (AdditionalRecordCount * Descrip= torSize) calculated - below. On output, it is the size= of new MemoryMap - after split. - @param MemoryMap A pointer to the buffer in which= firmware places - the current memory map. - @param DescriptorSize Size, in bytes, of an individual= EFI_MEMORY_DESCRIPTOR. - @param ImageRecordList A list of IMAGE_PROPERTIES_RECOR= D entries used when searching - for an image record contained by= the memory range described in - EFI memory map descriptors. - @param NumberOfAdditionalDescriptors The number of unused descriptors= at the end of the input MemoryMap. + Given an image record list, the caller should use the following formula = when allocating extra descriptors: + NumberOfAdditionalDescriptors =3D (MemoryMapSize / DescriptorSize) + + ((2 * + 3) * ) + + @param[in, out] MemoryMapSize IN: The size, in bytes= , of the old memory map before the split. + OUT: The size, in bytes= , of the used descriptors of the split + memory map + @param[in, out] MemoryMap IN: A pointer to the b= uffer containing the current memory map. + This buffer must h= ave enough space to accomodate the "worst case" + scenario where eve= ry image in ImageRecordList needs a new descriptor + to describe its co= de and data sections. + OUT: A pointer to the u= pdated memory map with separated image section + descriptors. + @param[in] DescriptorSize The size, in bytes, of a= n individual EFI_MEMORY_DESCRIPTOR. + @param[in] ImageRecordList A list of IMAGE_PROPERTI= ES_RECORD entries used when searching + for an image record cont= ained by the memory range described in + EFI memory map descripto= rs. + @param[in] NumberOfAdditionalDescriptors The number of unused des= criptors at the end of the input MemoryMap. + The formula in the descr= iption should be used to calculate this value. + + @retval EFI_SUCCESS The memory map was succe= ssfully split. + @retval EFI_INVALID_PARAMETER MemoryMapSize, MemoryMap= , or ImageRecordList was NULL. **/ -VOID +EFI_STATUS EFIAPI SplitTable ( IN OUT UINTN *MemoryMapSize, @@ -468,6 +458,10 @@ SplitTable ( UINTN RealSplitRecordCount; UINTN TotalSkippedRecords; =20 + if ((MemoryMapSize =3D=3D NULL) || (MemoryMap =3D=3D NULL) || (ImageReco= rdList =3D=3D NULL)) { + return EFI_INVALID_PARAMETER; + } + TotalSkippedRecords =3D 0; // // Let old record point to end of valid MemoryMap buffer. @@ -518,16 +512,19 @@ SplitTable ( =20 *MemoryMapSize =3D (IndexNewStarting - IndexNew - TotalSkippedRecords) *= DescriptorSize; =20 - return; + return EFI_SUCCESS; } =20 /** - Swap two code sections in image record. + Swap two code sections in a single IMAGE_PROPERTIES_RECORD. =20 - @param FirstImageRecordCodeSection first code section in image record - @param SecondImageRecordCodeSection second code section in image reco= rd + @param[in] FirstImageRecordCodeSection The first code section + @param[in] SecondImageRecordCodeSection The second code section + + @retval EFI_SUCCESS The code sections were swappe= d successfully + @retval EFI_INVALID_PARAMETER FirstImageRecordCodeSection o= r SecondImageRecordCodeSection is NULL **/ -VOID +EFI_STATUS EFIAPI SwapImageRecordCodeSection ( IN IMAGE_PROPERTIES_RECORD_CODE_SECTION *FirstImageRecordCodeSection, @@ -536,6 +533,10 @@ SwapImageRecordCodeSection ( { IMAGE_PROPERTIES_RECORD_CODE_SECTION TempImageRecordCodeSection; =20 + if ((FirstImageRecordCodeSection =3D=3D NULL) || (SecondImageRecordCodeS= ection =3D=3D NULL)) { + return EFI_INVALID_PARAMETER; + } + TempImageRecordCodeSection.CodeSegmentBase =3D FirstImageRecordCodeSecti= on->CodeSegmentBase; TempImageRecordCodeSection.CodeSegmentSize =3D FirstImageRecordCodeSecti= on->CodeSegmentSize; =20 @@ -544,19 +545,26 @@ SwapImageRecordCodeSection ( =20 SecondImageRecordCodeSection->CodeSegmentBase =3D TempImageRecordCodeSec= tion.CodeSegmentBase; SecondImageRecordCodeSection->CodeSegmentSize =3D TempImageRecordCodeSec= tion.CodeSegmentSize; + + return EFI_SUCCESS; } =20 /** - Sort code section in image record, based upon CodeSegmentBase from low t= o high. + Sort the code sections in the input ImageRecord based upon CodeSegmentBa= se from low to high. =20 - @param ImageRecord image record to be sorted + @param[in] ImageRecord IMAGE_PROPERTIES_RECORD to be sorted + + @retval EFI_SUCCESS The code sections in the input ImageReco= rd were sorted successfully + @retval EFI_ABORTED An error occurred while sorting the code= sections in the input ImageRecord + @retval EFI_INVALID_PARAMETER ImageRecord is NULL **/ -VOID +EFI_STATUS EFIAPI SortImageRecordCodeSection ( IN IMAGE_PROPERTIES_RECORD *ImageRecord ) { + EFI_STATUS Status; IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection; IMAGE_PROPERTIES_RECORD_CODE_SECTION *NextImageRecordCodeSection; LIST_ENTRY *ImageRecordCodeSectionLink; @@ -564,6 +572,10 @@ SortImageRecordCodeSection ( LIST_ENTRY *ImageRecordCodeSectionEndLink; LIST_ENTRY *ImageRecordCodeSectionList; =20 + if (ImageRecord =3D=3D NULL) { + return EFI_INVALID_PARAMETER; + } + ImageRecordCodeSectionList =3D &ImageRecord->CodeSegmentList; =20 ImageRecordCodeSectionLink =3D ImageRecordCodeSectionList->ForwardLi= nk; @@ -584,7 +596,11 @@ SortImageRecordCodeSection ( IMAGE_PROPERTIES_RECORD_CODE_SECTION_= SIGNATURE ); if (ImageRecordCodeSection->CodeSegmentBase > NextImageRecordCodeSec= tion->CodeSegmentBase) { - SwapImageRecordCodeSection (ImageRecordCodeSection, NextImageRecor= dCodeSection); + Status =3D SwapImageRecordCodeSection (ImageRecordCodeSection, Nex= tImageRecordCodeSection); + if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); + return EFI_ABORTED; + } } =20 NextImageRecordCodeSectionLink =3D NextImageRecordCodeSectionLink->F= orwardLink; @@ -593,15 +609,20 @@ SortImageRecordCodeSection ( ImageRecordCodeSectionLink =3D ImageRecordCodeSectionLink->Forward= Link; NextImageRecordCodeSectionLink =3D ImageRecordCodeSectionLink->Forward= Link; } + + return EFI_SUCCESS; } =20 /** - Check if code section in image record is valid. + Check if the code sections in the input ImageRecord are valid. + The code sections are valid if they don't overlap, are contained + within the the ImageRecord's ImageBase and ImageSize, and are + contained within the MAX_ADDRESS. =20 - @param ImageRecord image record to be checked + @param[in] ImageRecord IMAGE_PROPERTIES_RECORD to be checked =20 - @retval TRUE image record is valid - @retval FALSE image record is invalid + @retval TRUE The code sections in the input ImageRecord are valid + @retval FALSE The code sections in the input ImageRecord are invalid **/ BOOLEAN EFIAPI @@ -615,6 +636,10 @@ IsImageRecordCodeSectionValid ( LIST_ENTRY *ImageRecordCodeSectionEndLink; LIST_ENTRY *ImageRecordCodeSectionList; =20 + if (ImageRecord =3D=3D NULL) { + return FALSE; + } + DEBUG ((DEBUG_VERBOSE, "ImageCode SegmentCount - 0x%x\n", ImageRecord->C= odeSegmentCount)); =20 ImageRecordCodeSectionList =3D &ImageRecord->CodeSegmentList; @@ -661,10 +686,13 @@ IsImageRecordCodeSectionValid ( /** Swap two image records. =20 - @param FirstImageRecord first image record. - @param SecondImageRecord second image record. + @param[in] FirstImageRecord The first image record. + @param[in] SecondImageRecord The second image record. + + @retval EFI_SUCCESS The image records were swapped successful= ly + @retval EFI_INVALID_PARAMETER FirstImageRecord or SecondImageRecord is = NULL **/ -VOID +EFI_STATUS EFIAPI SwapImageRecord ( IN IMAGE_PROPERTIES_RECORD *FirstImageRecord, @@ -673,6 +701,10 @@ SwapImageRecord ( { IMAGE_PROPERTIES_RECORD TempImageRecord; =20 + if ((FirstImageRecord =3D=3D NULL) || (SecondImageRecord =3D=3D NULL)) { + return EFI_INVALID_PARAMETER; + } + TempImageRecord.ImageBase =3D FirstImageRecord->ImageBase; TempImageRecord.ImageSize =3D FirstImageRecord->ImageSize; TempImageRecord.CodeSegmentCount =3D FirstImageRecord->CodeSegmentCount; @@ -686,14 +718,19 @@ SwapImageRecord ( SecondImageRecord->CodeSegmentCount =3D TempImageRecord.CodeSegmentCount; =20 SwapListEntries (&FirstImageRecord->CodeSegmentList, &SecondImageRecord-= >CodeSegmentList); + return EFI_SUCCESS; } =20 /** - Sort image record based upon the ImageBase from low to high. + Sort the input ImageRecordList based upon the ImageBase from low to high. =20 - @param ImageRecordList Image record list to be sorted + @param[in] ImageRecordList Image record list to be sorted + + @retval EFI_SUCCESS The image record list was sorted successfu= lly + @retval EFI_ABORTED An error occurred while sorting the image = record list + @retval EFI_INVALID_PARAMETER ImageRecordList is NULL **/ -VOID +EFI_STATUS EFIAPI SortImageRecord ( IN LIST_ENTRY *ImageRecordList @@ -704,6 +741,11 @@ SortImageRecord ( LIST_ENTRY *ImageRecordLink; LIST_ENTRY *NextImageRecordLink; LIST_ENTRY *ImageRecordEndLink; + EFI_STATUS Status; + + if (ImageRecordList =3D=3D NULL) { + return EFI_INVALID_PARAMETER; + } =20 ImageRecordLink =3D ImageRecordList->ForwardLink; NextImageRecordLink =3D ImageRecordLink->ForwardLink; @@ -724,7 +766,11 @@ SortImageRecord ( =20 ); if (ImageRecord->ImageBase > NextImageRecord->ImageBase) { - SwapImageRecord (ImageRecord, NextImageRecord); + Status =3D SwapImageRecord (ImageRecord, NextImageRecord); + if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); + return EFI_ABORTED; + } } =20 NextImageRecordLink =3D NextImageRecordLink->ForwardLink; @@ -733,16 +779,20 @@ SortImageRecord ( ImageRecordLink =3D ImageRecordLink->ForwardLink; NextImageRecordLink =3D ImageRecordLink->ForwardLink; } + + return EFI_SUCCESS; } =20 /** Find image record according to image base and size. =20 - @param ImageBase Base of PE image - @param ImageSize Size of PE image - @param ImageRecordList Image record list to be searched + @param[in] ImageBase Base of PE image + @param[in] ImageSize Size of PE image + @param[in] ImageRecordList Image record list to be searched =20 - @return image record + @retval NULL No IMAGE_PROPERTIES_RECORD matches ImageBase + and ImageSize in the input ImageRecordList + @retval Other The found IMAGE_PROPERTIES_RECORD **/ IMAGE_PROPERTIES_RECORD * EFIAPI @@ -755,6 +805,10 @@ FindImageRecord ( IMAGE_PROPERTIES_RECORD *ImageRecord; LIST_ENTRY *ImageRecordLink; =20 + if (ImageRecordList =3D=3D NULL) { + return NULL; + } + for (ImageRecordLink =3D ImageRecordList->ForwardLink; ImageRecordLink !=3D ImageRecordList; ImageRecordLink =3D ImageRecordLink->ForwardLink) diff --git a/MdeModulePkg/Include/Library/ImagePropertiesRecordLib.h b/MdeM= odulePkg/Include/Library/ImagePropertiesRecordLib.h index 9139c7b6c05d..b6365662646a 100644 --- a/MdeModulePkg/Include/Library/ImagePropertiesRecordLib.h +++ b/MdeModulePkg/Include/Library/ImagePropertiesRecordLib.h @@ -32,56 +32,47 @@ typedef struct { } IMAGE_PROPERTIES_RECORD; =20 /** - Split the original memory map, and add more entries to describe PE code = section and data section. - This function will set EfiRuntimeServicesData to be EFI_MEMORY_XP. - This function will merge entries with same attributes finally. + Split the original memory map and add more entries to describe PE code + and data sections for each image in the input ImageRecordList. =20 - NOTE: It assumes PE code/data section are page aligned. - NOTE: It assumes enough entry is prepared for new memory map. + NOTE: This function assumes PE code/data section are page aligned. + NOTE: This function assumes there are enough entries for the new memory = map. =20 - Split table: - +---------------+ - | Record X | - +---------------+ - | Record RtCode | - +---------------+ - | Record Y | - +---------------+ - =3D=3D> - +---------------+ - | Record X | - +---------------+ ---- - | Record RtData | | - +---------------+ | - | Record RtCode | |-> PE/COFF1 - +---------------+ | - | Record RtData | | - +---------------+ ---- - | Record RtData | | - +---------------+ | - | Record RtCode | |-> PE/COFF2 - +---------------+ | - | Record RtData | | - +---------------+ ---- - | Record Y | - +---------------+ + | | | | | | | | + | 4K PAGE | DATA | CODE | DATA | CODE | DATA | 4K PAGE | + | | | | | | | | + Assume the above memory region is the result of one split memory map des= criptor. It's unlikely + that a linker will orient an image this way, but the caller must assume = the worst case scenario. + This image layout example contains code sections oriented in a way that = maximizes the number of + descriptors which would be required to describe each section. To ensure = we have enough space + for every descriptor of the broken up memory map, the caller must assume= that every image will + have the maximum number of code sections oriented in a way which maximiz= es the number of data + sections with unrelated memory regions flanking each image within a sing= le descriptor. =20 - @param MemoryMapSize A pointer to the size, in bytes,= of the - MemoryMap buffer. On input, this= is the size of - old MemoryMap before split. The = actual buffer - size of MemoryMap is MemoryMapSi= ze + - (AdditionalRecordCount * Descrip= torSize) calculated - below. On output, it is the size= of new MemoryMap - after split. - @param MemoryMap A pointer to the buffer in which= firmware places - the current memory map. - @param DescriptorSize Size, in bytes, of an individual= EFI_MEMORY_DESCRIPTOR. - @param ImageRecordList A list of IMAGE_PROPERTIES_RECOR= D entries used when searching - for an image record contained by= the memory range described in - EFI memory map descriptors. - @param NumberOfAdditionalDescriptors The number of unused descriptors= at the end of the input MemoryMap. + Given an image record list, the caller should use the following formula = when allocating extra descriptors: + NumberOfAdditionalDescriptors =3D (MemoryMapSize / DescriptorSize) + + ((2 * + 3) * ) + + @param[in, out] MemoryMapSize IN: The size, in bytes= , of the old memory map before the split. + OUT: The size, in bytes= , of the used descriptors of the split + memory map + @param[in, out] MemoryMap IN: A pointer to the b= uffer containing the current memory map. + This buffer must h= ave enough space to accomodate the "worst case" + scenario where eve= ry image in ImageRecordList needs a new descriptor + to describe its co= de and data sections. + OUT: A pointer to the u= pdated memory map with separated image section + descriptors. + @param[in] DescriptorSize The size, in bytes, of a= n individual EFI_MEMORY_DESCRIPTOR. + @param[in] ImageRecordList A list of IMAGE_PROPERTI= ES_RECORD entries used when searching + for an image record cont= ained by the memory range described in + EFI memory map descripto= rs. + @param[in] NumberOfAdditionalDescriptors The number of unused des= criptors at the end of the input MemoryMap. + The formula in the descr= iption should be used to calculate this value. + + @retval EFI_SUCCESS The memory map was succe= ssfully split. + @retval EFI_INVALID_PARAMETER MemoryMapSize, MemoryMap= , or ImageRecordList was NULL. **/ -VOID +EFI_STATUS EFIAPI SplitTable ( IN OUT UINTN *MemoryMapSize, @@ -92,23 +83,30 @@ SplitTable ( ); =20 /** - Sort code section in image record, based upon CodeSegmentBase from low t= o high. + Sort the code sections in the input ImageRecord based upon CodeSegmentBa= se from low to high. =20 - @param ImageRecord image record to be sorted + @param[in] ImageRecord IMAGE_PROPERTIES_RECORD to be sorted + + @retval EFI_SUCCESS The code sections in the input ImageReco= rd were sorted successfully + @retval EFI_ABORTED An error occurred while sorting the code= sections in the input ImageRecord + @retval EFI_INVALID_PARAMETER ImageRecord is NULL **/ -VOID +EFI_STATUS EFIAPI SortImageRecordCodeSection ( IN IMAGE_PROPERTIES_RECORD *ImageRecord ); =20 /** - Check if code section in image record is valid. + Check if the code sections in the input ImageRecord are valid. + The code sections are valid if they don't overlap, are contained + within the the ImageRecord's ImageBase and ImageSize, and are + contained within the MAX_ADDRESS. =20 - @param ImageRecord image record to be checked + @param[in] ImageRecord IMAGE_PROPERTIES_RECORD to be checked =20 - @retval TRUE image record is valid - @retval FALSE image record is invalid + @retval TRUE The code sections in the input ImageRecord are valid + @retval FALSE The code sections in the input ImageRecord are invalid **/ BOOLEAN EFIAPI @@ -117,11 +115,15 @@ IsImageRecordCodeSectionValid ( ); =20 /** - Sort image record based upon the ImageBase from low to high. + Sort the input ImageRecordList based upon the ImageBase from low to high. =20 - @param ImageRecordList Image record list to be sorted + @param[in] ImageRecordList Image record list to be sorted + + @retval EFI_SUCCESS The image record list was sorted successfu= lly + @retval EFI_ABORTED An error occurred while sorting the image = record list + @retval EFI_INVALID_PARAMETER ImageRecordList is NULL **/ -VOID +EFI_STATUS EFIAPI SortImageRecord ( IN LIST_ENTRY *ImageRecordList @@ -132,8 +134,11 @@ SortImageRecord ( =20 @param[in] FirstImageRecord The first image record. @param[in] SecondImageRecord The second image record. + + @retval EFI_SUCCESS The image records were swapped successful= ly + @retval EFI_INVALID_PARAMETER FirstImageRecord or SecondImageRecord is = NULL **/ -VOID +EFI_STATUS EFIAPI SwapImageRecord ( IN IMAGE_PROPERTIES_RECORD *FirstImageRecord, @@ -145,8 +150,11 @@ SwapImageRecord ( =20 @param[in] FirstImageRecordCodeSection The first code section @param[in] SecondImageRecordCodeSection The second code section + + @retval EFI_SUCCESS The code sections were swappe= d successfully + @retval EFI_INVALID_PARAMETER FirstImageRecordCodeSection o= r SecondImageRecordCodeSection is NULL **/ -VOID +EFI_STATUS EFIAPI SwapImageRecordCodeSection ( IN IMAGE_PROPERTIES_RECORD_CODE_SECTION *FirstImageRecordCodeSection, @@ -154,13 +162,16 @@ SwapImageRecordCodeSection ( ); =20 /** - Find image record according to image base and size. + Find image properties record according to image base and size in the + input ImageRecordList. =20 - @param ImageBase Base of PE image - @param ImageSize Size of PE image - @param ImageRecordList Image record list to be searched + @param[in] ImageBase Base of PE image + @param[in] ImageSize Size of PE image + @param[in] ImageRecordList Image record list to be searched =20 - @return image record + @retval NULL No IMAGE_PROPERTIES_RECORD matches ImageBase + and ImageSize in the input ImageRecordList + @retval Other The found IMAGE_PROPERTIES_RECORD **/ IMAGE_PROPERTIES_RECORD * EFIAPI --=20 2.42.0.windows.2 -=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 (#110658): https://edk2.groups.io/g/devel/message/110658 Mute This Topic: https://groups.io/mt/102368850/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-