Re: [edk2-devel] [PATCH v1] MdeModulePkg: Fixup MAT Attributes After Splitting EFI Memory Map

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

On Wed, 17 Apr 2024 at 16:34, Taylor Beebe <taylor.d.beebe@gmail.com> wrote:
>
>
> On 4/17/2024 7:09 AM, Oliver Smith-Denny wrote:
> > On 4/17/2024 7:05 AM, Taylor Beebe wrote:
> >>
> >> On 4/17/2024 6:40 AM, Oliver Smith-Denny wrote:
> >>> Aside from this, I wonder if we can be more aspirational here. These
> >>> EfiRuntimeServicesCode regions without attributes set are, if I am
> >>> understanding correctly, from loaded images.
> >> These EfiRuntimeServicesCode regions without attributes set are
> >> not part of loaded image memory. I think that's what you meant but
> >> wanted to clarify.
> >
> > Are these regions without attributes from image sections that have
> > been padded to RUNTIME_PAGE_ALLOCATION_GRANULARITY, i.e. they are
> > the pads? Or are we saying we don't know what these regions are
> > at this point? It is true in theory someone could just allocate
> > an EfiRuntimeServicesCode section.
> Good question -- I had not considered the extra padding applied
> to these allocations. It could be either. The memory map returned
> via GetMemoryMap() will merge descriptors together based on type
> so it's possible to mistake an unrelated EfiRuntimeServicesCode
> allocation with padding applied to a runtime image memory
> allocation if they are contiguous.
>

An arbitary Linux/arm64 on edk2 VM boot gives me

memory map:
...
0x00007c770000-0x00007c7effff [Runtime Code|RUN|  |  |  |  |  |  |  |  ]
0x00007c7f0000-0x00007c95ffff [Runtime Data|RUN|  |  |  |  |  |  |  |  ]
0x00007c960000-0x00007c9affff [Runtime Code|RUN|  |  |  |  |  |  |  |  ]
0x00007c9b0000-0x00007ca4ffff [Runtime Data|RUN|  |  |  |  |  |  |  |  ]
0x00007ca50000-0x00007cb3ffff [Runtime Code|RUN|  |  |  |  |  |  |  |  ]
0x00007cb40000-0x00007cb42fff [Conventional|   |  |  |  |  |  |  |  |  ]
0x00007cb43000-0x00007cb43fff [Loader Data |   |  |  |  |  |  |  |  |  ]
0x00007cb44000-0x00007ecd5fff [Conventional|   |  |  |  |  |  |  |  |  ]
0x00007ecd6000-0x00007fa37fff [Boot Data   |   |  |  |  |  |  |  |  |  ]
0x00007fa38000-0x00007faa0fff [Conventional|   |  |  |  |  |  |  |  |  ]
0x00007faa1000-0x00007fe1ffff [Boot Code   |   |  |  |  |  |  |  |  |  ]
0x00007fe20000-0x00007feaffff [Runtime Code|RUN|  |  |  |  |  |  |  |  ]
0x00007feb0000-0x00007febffff [Conventional|   |  |  |  |  |  |  |  |  ]
0x00007fec0000-0x00007ffdffff [Runtime Data|RUN|  |  |  |  |  |  |  |  ]
...

MAT:
0x00007c770000-0x00007c77ffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007c780000-0x00007c78ffff [Runtime Code|RUN|  |  |  |  |  |  |  |RO]
0x00007c790000-0x00007c7bffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007c7c0000-0x00007c7cffff [Runtime Code|RUN|  |  |  |  |  |  |  |RO]
0x00007c7d0000-0x00007c7effff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007c7f0000-0x00007c95ffff [Runtime Data|RUN|  |  |  |  |XP|  |  |  ]
0x00007c960000-0x00007c96ffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007c970000-0x00007c97ffff [Runtime Code|RUN|  |  |  |  |  |  |  |RO]
0x00007c980000-0x00007c9affff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007c9b0000-0x00007ca4ffff [Runtime Data|RUN|  |  |  |  |XP|  |  |  ]
0x00007ca50000-0x00007ca5ffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007ca60000-0x00007ca6ffff [Runtime Code|RUN|  |  |  |  |  |  |  |RO]
0x00007ca70000-0x00007caaffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007cab0000-0x00007cabffff [Runtime Code|RUN|  |  |  |  |  |  |  |RO]
0x00007cac0000-0x00007cafffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007cb00000-0x00007cb0ffff [Runtime Code|RUN|  |  |  |  |  |  |  |RO]
0x00007cb10000-0x00007cb3ffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007fe20000-0x00007fe2ffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007fe30000-0x00007fe3ffff [Runtime Code|RUN|  |  |  |  |  |  |  |RO]
0x00007fe40000-0x00007fe6ffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007fe70000-0x00007fe7ffff [Runtime Code|RUN|  |  |  |  |  |  |  |RO]
0x00007fe80000-0x00007feaffff [Runtime Code|RUN|  |  |  |  |XP|  |  |  ]
0x00007fec0000-0x00007ffdffff [Runtime Data|RUN|  |  |  |  |XP|  |  |  ]

So there are far fewer entries in the memory map than in the MAT, due
to the merging. AFAICT, there are ~8 runtime DXEs here.

I imagine other Rt Code regions could easily be get merged in here as well.

The wording about describing the larger region entirely is
unfortunate: I was involved in the creation of the MAT, and this
merging was definitely not taken into account.

The MAT replaced a feature where runtime DXE images were split into
separate Rt code and date memmap entries based on the PE sections, and
this broke spectacularly due to the fact that SetVirtualAddressMap()
may reorder those regions in virtual memory, which breaks relative
references between code and data in those runtime DXE programs.

So the intent has always been to describe memory attributes at a
higher level of detail without relying on the PE/COFF header directly
(which principle went out the window with PRM but that is another
discussion)

So the purpose of the MAT is to describe RT code (and to a lesser
extent, RT data) regions where we cannot apply either RO or XP to the
whole thing. IIRC there was never an intent to exhaustively describe
all memory runtime regions. Also note that RO was introduced at this
point, because WP was already being used in the ordinary memory map in
a deviating manner. RO is defined both for the memory map and the MAT,
and so it can occur in either.


> When the IMAGE_PROPERTIES_RECORD entry is created, perhaps
> it would be best to set the ImageSize field to the padded allocation
> size instead of the file size. Is this the difference between virtual size
> and raw data size? I recall you recently did this in
> ImagePropertiesRecordLib
> for the code size of a new entry.
>

Not sure whether the padding should matter tbh. All these entries
should describe the situation after the PE image was rounded outwards
to the minimum runtime alignment.


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