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 VTableCallHolder writeable mapping size with W^X #70093

Merged
merged 1 commit into from
Jun 1, 2022

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Jun 1, 2022

The size of this holder is dynamic, but when we are creating the writeable
mapping of this holder to initialize its code, we don't take that into account.
So in case the holder is located at the end of a memory page and crosses its
boundary, the writeable mapping covers only the beginning of the holder and
so we either crash during the initialization if the following memory page is
not mapped or read only, or we corrupt a completely unrelated memory page
in case it is mapped and writeable.

The fix is to use the real size of the holder instead of sizeof(...).

The size of this holder is dynamic, but when we are creating the writeable
mapping of this holder to initialize its code, we don't take that into account.
So in case the holder is located at the end of a memory page and crosses its
boundary, the writeable mapping covers only the beginning of the holder and
so we either crash during the initialization if the following memory page is
not mapped or read only, or we corrupt a completely unrelated memory page
in case it is mapped and writeable.

The fix is to use the real size of the holder instead of sizeof(...).
@janvorli janvorli added this to the 7.0.0 milestone Jun 1, 2022
@janvorli janvorli requested a review from jkotas June 1, 2022 15:52
@janvorli janvorli self-assigned this Jun 1, 2022
@mmitche
Copy link
Member

mmitche commented Jun 1, 2022

@janvorli Seeing some new (though I think inconsistent) failures in the sdk->installer build for p5 that could be a result of this. May need a backport.

dotnet/installer#13881

@janvorli
Copy link
Member Author

janvorli commented Jun 1, 2022

@mmitche I have made this fix based on investigations of the installer issue for p6, I was going to port this to p5 as the issue was there too.

@janvorli janvorli merged commit 9d84900 into dotnet:main Jun 1, 2022
@janvorli
Copy link
Member Author

janvorli commented Jun 1, 2022

/backport to release/7.0-preview5

@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2022

Started backporting to release/7.0-preview5: https://github.com/dotnet/runtime/actions/runs/2423701222

@ghost ghost locked as resolved and limited conversation to collaborators Jul 1, 2022
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.

4 participants