Hi Ard,
On 3/1/2024 3:58 AM, Ard Biesheuvel wrote:
Hi Oliver,
On Tue, 27 Feb 2024 at 21:27, Oliver Smith-Denny
<o...@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?
This is an artifact of PE/COFF itself and is useful for having smaller
FW images. In PE/COFF we have the section alignment (which is how we get
loaded into memory) and the file alignment (how the actual file is
aligned). It is valid for these two to be different. For example, these
runtime DXE drivers, which are not XIP (which the case we do need the
section and file alignment to be the same, as we are executing from
the file image) can be a smaller size in the file, but when loaded into
memory we will relocate them and do the proper rebasing to set these on
64k boundaries. Different toolchains have different ways of specifying
the two things, on gcc I have seen common-page-size affect the section
alignment and max-page-size affect both section and file alignment. For
msvc there are /ALIGN and /FILEALIGN commands which directly manipulate
these values.
The issue here was not that we have different section and file
alignment, in fact, the issue still exists if section alignement ==
file alignment. This is because SizeOfRawData in the PE/COFF header is
the raw size of bytes used, not even page aligned. So no matter what,
the image records we were creating here had bad sizes being set which
do not match what the image loader was actually doing.
I do think there is a fair argument that would say the image loader
should create the image records when it loads images, since in the
end we want the record to match exactly what the image in memory is,
creating after the fact is a poor pattern.
Cc: Liming Gao <gaolim...@byosoft.com.cn>
Cc: Leif Lindholm <quic_llind...@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
Cc: Sami Mujawar <sami.muja...@arm.com>
Cc: Taylor Beebe <taylor.d.be...@gmail.com>
Signed-off-by: Oliver Smith-Denny <o...@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?
Correct, SectionAlignment is the alignment of the image as loaded in
memory, so in the case of a DXE runtime driver on ARM64, it will be
64k.
InsertTailList (&ImageRecord->CodeSegmentList,
&ImageRecordCodeSection->Link);
ImageRecord->CodeSegmentCount++;
--
2.40.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116334): https://edk2.groups.io/g/devel/message/116334
Mute This Topic: https://groups.io/mt/104610770/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
