search

microsoft / mu_plus

Project Mu Microsoft Core UEFI Value
https://microsoft.github.io/mu/
Other
211 stars 99 forks source link

[Bug]: MemoryOutsideEfiMemoryMapIsInaccessible() Test Is Broken #531

Closed TaylorBeebe closed 1 month ago

TaylorBeebe commented 1 month ago

Is there an existing issue for this?

Current Behavior

PR 528 updated the MemoryOutsideEfiMemoryMapIsInaccessible() test to check the return status of ValidateRegionAttributes(). ValidateRegionAttributes() returns a boolean, not a status. So, interpreting the FALSE return value is being mistaken for an EFI_SUCCESS return value.

I know, I know... usually we return a status... SOMETIMES I COLORED OUTSIDE THE LINES 😅

Expected Behavior

The AllowUnmappedRegions argument of ValidateRegionAttributes() dictates if EFI_NO_MAPPING is acceptable for the region. So, the MemoryOutsideEfiMemoryMapIsInaccessible() test was already checking for EFI_NO_MAPPING but now the test will only pass if the required conditions are not met.

Steps To Reproduce

N/A

Build Environment

N/A

Version Information

69e9464117eff84e88b9646c0e9c2e95a807fe60

Urgency

Low

Are you going to fix this?

Someone else needs to fix it

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

Miss y'all ❤️ Hope everything is going well 😃

TaylorBeebe commented 1 month ago

@apop5 @makubacki @os-d

os-d commented 1 month ago

@TaylorBeebe, ha, thanks for catching this. I will fix it.

TaylorBeebe commented 1 month ago

@os-d

No problem 😄

If you want this test to pass, I think the first step is to look into removing the following lines:

  1. https://github.com/microsoft/mu_basecore/blob/856d8a412308b852e53254c10edd0f098933aa8f/MdeModulePkg/Core/Dxe/Misc/MemoryProtectionSupport.c#L1186
  2. https://github.com/microsoft/mu_basecore/blob/856d8a412308b852e53254c10edd0f098933aa8f/MdeModulePkg/Core/Dxe/Misc/MemoryProtectionSupport.c#L1221
  3. https://github.com/microsoft/mu_basecore/blob/856d8a412308b852e53254c10edd0f098933aa8f/MdeModulePkg/Core/Dxe/Misc/MemoryProtectionSupport.c#L1257

If I remember correctly, one of the issues with removing the above lines is detailed in our Teams chat toward when you joined the team. The project led by Aaron hits a different issue if the above lines are removed which I think he remembers.

I'm pretty limited in what support I can provide, but feel free to reach out if questions arise.