Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
TaylorBeebe opened this issue Jul 27, 2024 · 3 comments · Fixed by #532
Closed
1 task done

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

TaylorBeebe opened this issue Jul 27, 2024 · 3 comments · Fixed by #532
Labels
state:needs-owner Needs an issue owner to be assigned state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working urgency:low Little to no impact

Comments

@TaylorBeebe
Copy link
Contributor

Is there an existing issue for this?

  • I have searched existing issues

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 TaylorBeebe added state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working labels Jul 27, 2024
@github-actions github-actions bot added urgency:low Little to no impact state:needs-owner Needs an issue owner to be assigned labels Jul 27, 2024
@TaylorBeebe
Copy link
Contributor Author

@apop5 @makubacki @os-d

@os-d
Copy link
Contributor

os-d commented Jul 27, 2024

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

@TaylorBeebe
Copy link
Contributor Author

@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.

@TaylorBeebe TaylorBeebe changed the title [Bug]: <MemoryOutsideEfiMemoryMapIsInaccessible() Test Is Broken> [Bug]: MemoryOutsideEfiMemoryMapIsInaccessible() Test Is Broken Jul 27, 2024
@os-d os-d closed this as completed Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:needs-owner Needs an issue owner to be assigned state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working urgency:low Little to no impact
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants