Re: [edk2-devel][PATCH v1 1/3] MdeModulePkg: ImagePropertiesRecordLib: Use SectionAlignment for CodeSize

["Ard Biesheuvel" <ardb@kernel.org>]

Hi Oliver,

On Tue, 27 Feb 2024 at 21:27, Oliver Smith-Denny
<osde@linux.microsoft.com> wrote:
>
> When an ImageRecord is stored by ImagePropertiesRecordLib, it reports the
> CodeSegmentSize as the SizeOfRawData from the image. However, the image
> as loaded into memory is aligned to the SectionAlignment, so
> SizeOfRawData is under the actual size in memory. This is important,
> because the memory attributes table uses these image records to create
> its entries and it will report that the alignment of an image is
> incorrect, even though the actual image is correct.
>
> This was discovered on ARM64, which has a 64k runtime page granularity
> alignment, which is backed by a 64k section alignment for
> DXE_RUNTIME_DRIVERs. The runtime code and data was correctly being
> loaded into memory, however the memory attribute table was incorrectly
> reporting misaligned ranges to the OS, causing attributes to be
> ignored for these sections for OSes using greater than 4k pages.
>
> This patch correctly aligns the CodeSegmentSize to the SectionAlignment
> and the corresponding memory attribute table entries are now correctly
> aligned and pointing to the right places in memory.
>

Can you explain how these can differ in the first place? Our flaky
ELF-to-PE/COFF converter should never generate such images to begin
with (which is probably how we ended up with this problem in the first
place), so I  suppose this is native PE/COFF tooling emitting sections
either using a non-1:1 file:memory mapping, or with unallocated holes
in the file representation?

> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Taylor Beebe <taylor.d.beebe@gmail.com>
>
> Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
> ---
>  MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
> index e53ce086c54c..07ced0e54e38 100644
> --- a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
> +++ b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
> @@ -1090,7 +1090,7 @@ CreateImagePropertiesRecord (
>        ImageRecordCodeSection->Signature = IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE;
>
>        ImageRecordCodeSection->CodeSegmentBase = (UINTN)ImageBase + Section[Index].VirtualAddress;
> -      ImageRecordCodeSection->CodeSegmentSize = Section[Index].SizeOfRawData;
> +      ImageRecordCodeSection->CodeSegmentSize = ALIGN_VALUE (Section[Index].SizeOfRawData, SectionAlignment);
>

This should be the virtual size, not the file size, right?

>        InsertTailList (&ImageRecord->CodeSegmentList, &ImageRecordCodeSection->Link);
>        ImageRecord->CodeSegmentCount++;
> --
> 2.40.1
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116237): https://edk2.groups.io/g/devel/message/116237
Mute This Topic: https://groups.io/mt/104610770/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-