Skip to content

Comments

dxe_core: Ensure Stack Guard Is Enabled When Init'ing Paging#1277

Merged
makubacki merged 1 commit intoOpenDevicePartnership:mainfrom
os-d:stackguard
Feb 9, 2026
Merged

dxe_core: Ensure Stack Guard Is Enabled When Init'ing Paging#1277
makubacki merged 1 commit intoOpenDevicePartnership:mainfrom
os-d:stackguard

Conversation

@os-d
Copy link
Contributor

@os-d os-d commented Feb 3, 2026

Description

Currently, the init_paging routine doesn't know about the stack guard, which is set up earlier. It ends up wiping this.

This commit moves the stack guard setup to init_paging to take it into account.

It was considered to make this more generic, to take already set attributes, but the number of cases where we expect access attributes to be set should just be the null page and stack guard page, so handle them explicitly.

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

Tested on Q35 and SBSA with the memory protections test failing and then succeeding.

Integration Instructions

N/A.

@os-d os-d added the type:bug Something isn't working label Feb 3, 2026
@github-actions github-actions bot added the impact:security Has a security impact label Feb 3, 2026
@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 78.57143% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
patina_dxe_core/src/gcd/spin_locked_gcd.rs 78.57% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@os-d os-d force-pushed the stackguard branch 2 times, most recently from 5ea9b18 to bf2066a Compare February 7, 2026 00:00
Currently, the init_paging routine doesn't know about the stack
guard, which is set up earlier. It ends up wiping this.

This commit moves the stack guard setup to init_paging to take
it into account.

It was considered to make this more generic, to take already
set attributes, but the number of cases where we expect access
attributes to be set should just be the null page and stack
guard page, so handle them explicitly.

Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
@os-d
Copy link
Contributor Author

os-d commented Feb 7, 2026

@makubacki can we force merge this? It’s actually more code coverage than it was before, but I only got it to 78% patch coverage. I could force the attribute setting to fail on the stack hob, but that’s not useful to test, it’s just a panic.

@makubacki makubacki merged commit 2a69561 into OpenDevicePartnership:main Feb 9, 2026
7 of 8 checks passed
@os-d os-d deleted the stackguard branch February 9, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact:security Has a security impact type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants