Skip to content

Conversation

@janvorli
Copy link
Member

The RtlRestoreContext sets SP before reading out PC from the
context. That can lead to a corruption of the PC in the context
if an async signal is delivered to the thread or the thread is
interrupted by any other mean after the SP is set and before
the value of PC is extracted from the context.

This change fixes it by setting the SP after both PC and SP values
are read from the context data structure.

The RtlRestoreContext sets SP before reading out PC from the
context. That can lead to a corruption of the PC in the context
if an async signal is delivered to the thread  or the thread is
interrupted by any other mean after the SP is set and before
the value of PC is extracted from the context.

This change fixes it by setting the SP after both PC and SP values
are read from the context data structure.
@janvorli janvorli requested a review from jkotas September 30, 2021 21:38
@janvorli janvorli self-assigned this Sep 30, 2021
ldr x17, [x16, CONTEXT_Sp]
mov sp, x17
ldr x17, [x16, CONTEXT_Pc]
ldp x16, x17, [x16, CONTEXT_Sp] // Context_Pc is right after Context_Sp
Copy link
Member

Choose a reason for hiding this comment

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

Neat!

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you! I think we should consider this for backport to .NET 6.

@jkotas jkotas merged commit 900a0d6 into dotnet:main Oct 1, 2021
@jkotas
Copy link
Member

jkotas commented Oct 1, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Oct 1, 2021

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1293350876

@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants