Hi Ard, On 4/16/2024 11:38 PM, Ard Biesheuvel wrote:
For entries where we lack such additional metadata, I don't think we can make assumptions based on the type beyond mapping data and MMIO regions XP. We have no idea how those EfiRuntimeServicesCode regions may be used, and currently, the spec permits RWX semantics for those if no restrictions are specified. The spec suggests that omitting an entry from the MAT is the same as listing it with RO|XP cleared. How RO|XP from the original entry should be interpreted wrt to the MAT is unspecified. So I think the only thing we can do at this point is preserve the entry if it has RO|XP set, or just drop it otherwise.
I do agree that it is probably safer to exclude the sub-region from the MAT entirely. However, from my reading of the spec, it is unclear to me whether it envisions that. From section 4.6.4 of UEFI spec 2.10: "The Memory Attributes Table may define multiple entries to describe sub-regions that comprise a single entry returned by GetMemoryMap() however the sub-regions must total to completely describe the larger region and may not cross boundaries between entries reported by GetMemoryMap() . If a run-time region returned in GetMemoryMap() entry is not described within the Memory Attributes Table, this region is assumed to not be compatible with any memory protections." The unclear part to me here is "the sub-regions must total to completely describe the larger region." To me that says that in Taylor's case, where we have a memory map descriptor with attributes set for part of the region but not the whole region, that the spec envisions the MAT having either the whole region split into sub-regions or not including the MAT entry for this region. In this reading the final sentence would say that if an entire memory map entry is not described in the MAT, then it is assumed to be incompatible. A different reading would say what you are saying, that a sub-region can be dropped from the MAT (although it is hard to justify that with the language that says the sub-regions must total to completely describe the larger region). What are your thoughts here? 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. The spec does call out that EfiRuntimeServicesCode is explicitly for code sections of loaded images. Can we just say outright that any memory of this type should be RO? I understand that existing drivers may attempt to break this, but from the core, I think this is reasonable to say, but would love to hear thoughts.
Note that the spec also mentions that the MAT must only contain EfiRuntimeServicesCode or EfiRuntimeServicesData entries, and it looks like this is not being enforced either.
Agreed, this should be amended. Thanks, Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117920): https://edk2.groups.io/g/devel/message/117920 Mute This Topic: https://groups.io/mt/105570114/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
