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

fix: Allow Windows CFI to underflow #645

Merged
merged 2 commits into from
Aug 2, 2022
Merged

Conversation

Swatinem
Copy link
Member

@Swatinem Swatinem commented Aug 2, 2022

We have seen some SaveNonVolatile unwind operations underflow the current stack frame, so they are saving/loading registers into the callers stack frame.
For example VCRUNTIME140.dll!__C_specific_handler can do this.

Its full Unwind Operations look like this:

Function Table:
  Start Address: 0xbff0
  End Address: 0xc1eb
  Unwind Info Address: 0x10138
    Version: 1
    Flags: 0
    Size of prolog: 28
    Number of Codes: 12
    No frame pointer used
    Unwind Codes:
      0x1c: UOP_SaveNonVol RSI [0x0080]
      0x1c: UOP_SaveNonVol RBP [0x0078]
      0x1c: UOP_SaveNonVol RBX [0x0070]
      0x1c: UOP_AllocSmall 64
      0x18: UOP_PushNonVol R15
      0x16: UOP_PushNonVol R14
      0x14: UOP_PushNonVol R13
      0x12: UOP_PushNonVol R12
      0x10: UOP_PushNonVol RDI

We previously used saturating_sub to clamp these offsets to 0, meaning things are loaded from the current stack frame instead of from the callers frame. Now we correctly generate ASCII CFI for these Unwind Codes.

This fixes one pre-condition of #638, namely that it depends on $rbp which is loaded from the above unwind codes, thus outside its own stack frame.

We have seen some SaveNonVolatile unwind operations underflow the current stack frame, so they are saving/loading registers into the callers stack frame.
For example `VCRUNTIME140.dll!__C_specific_handler` can do this.

Its full Unwind Operations look like this:

```
Function Table:
  Start Address: 0xbff0
  End Address: 0xc1eb
  Unwind Info Address: 0x10138
    Version: 1
    Flags: 0
    Size of prolog: 28
    Number of Codes: 12
    No frame pointer used
    Unwind Codes:
      0x1c: UOP_SaveNonVol RSI [0x0080]
      0x1c: UOP_SaveNonVol RBP [0x0078]
      0x1c: UOP_SaveNonVol RBX [0x0070]
      0x1c: UOP_AllocSmall 64
      0x18: UOP_PushNonVol R15
      0x16: UOP_PushNonVol R14
      0x14: UOP_PushNonVol R13
      0x12: UOP_PushNonVol R12
      0x10: UOP_PushNonVol RDI
```

We previously used saturating_sub to clamp these offsets to 0, meaning things are loaded from the current stack frame instead of from the callers frame. Now we correctly generate ASCII CFI for these Unwind Codes.
@Swatinem Swatinem requested a review from a team August 2, 2022 13:28
@@ -1074,7 +1080,8 @@ impl<W: Write> AsciiCfiWriter<W> {
" {}: {} {} + ^",
reg.name(),
unwind_info.frame_register.name(),
offset.saturating_sub(unwind_info.frame_register_offset)
offset.wrapping_sub(unwind_info.frame_register_offset)
as i32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, thats ugly wrapping :3

Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snapshots + nit, but 💯

@Swatinem Swatinem enabled auto-merge (squash) August 2, 2022 14:06
@Swatinem Swatinem merged commit 2145379 into master Aug 2, 2022
@Swatinem Swatinem deleted the fix/wincfi-underflow branch August 2, 2022 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants