Set ARM RTC MMIO as NX

[os-d]

Description

Two ARM RTC libs will clear EFI_MEMORY_XP from MMIO memory if a platform has set it, because they explicitly set memory attributes that do not include EFI_MEMORY_XP. These regions are not intended to be executed from, so this patch set updates the libs to explicitly set the regions to EFI_MEMORY_XP.

• [ ] Breaking change?
  • Breaking change - Does this PR cause a break in build or boot behavior?
  • Examples: Does it add a new library class or move a module to a different repo.
• [x] Impacts security?
  • Security - Does this PR have a direct security impact?
  • Examples: Crypto algorithm change or buffer overflow fix.
• [ ] Includes tests?
  • Tests - Does this PR include any explicit test code?
  • Examples: Unit tests or integration tests.

How This Was Tested

Tested on ARM64 virtual platforms.

Integration Instructions

N/A.

[os-d]

@ardbiesheuvel @leiflindholm @samimujawar can you please review?

[ardbiesheuvel]

Does this actually make a difference in practice? Device mappings on ARM (v7 and later) are required to have the PXN/UXN attributes set anyway, as otherwise, speculative instruction fetches can (and will) access those regions spuriously.

Loss of interrupts due to the CPU's I-cache acking them behind your back -by attempting to speculatively fetch instructions from your GIC control registers- is a somewhat infamous example of what might go wrong, and so EDK2 should never create device mappings that are executable in practice.

Whether or not the GCD attributes reflect this is another matter, but I'd argue that the architecture specific semantics make the EFI_MEMORY_XP attribute a dont-care when EFI_MEMORY_UC is the only memory type bit set on the entry.

[os-d]

@ardbiesheuvel, I agree that in the page table this does not make a difference, as you pointed out when UC is set, we always set the PXN/UXN attributes. This is split out from a much larger memory protections patchset (which is disparate and over the course of years, hence splitting it apart) that we are in the process of upstreaming from Project Mu, so I purposefully split this one off as these are more trivial.

However, I do think it is important that the GCD have accurate attributes for two reasons: reporting accurately in the EFI_MEMORY_MAP to an OS and for auditability. For the first case, if we have runtime MMIO ranges, we should report that EFI_MEMORY_XP is set to the OS in the EFI_MEMORY_MAP. If the driver doesn't set them, we will not do so. You will correctly note that we are stripping all access attributes from the EFI_MEMORY_MAP today because of some >5 year old bugs in OSes. This is code that should not live forever and I think the GCD should be accurate in access attributes when we go to remove that.

For the auditability case, we have test apps that run to verify the GCD matches what we have in the page table and what we expect for a given memory type. We can drill into each case and figure out what wasn't setting bits correctly, but that's what led us to add change like this one :). So might as well cement it and not redo the manual auditing. Plus, one more spurious reason is for copy/paste. If we do the "right" thing here, if someone uses this as an example of how to set attributes, they will by default get setting XP :).