Skip to content

Fix VTableCallHolder writeable mapping size with W^X#70093

Merged
janvorli merged 1 commit into
dotnet:mainfrom
janvorli:fix-vtable-stub-holder-mapping-size
Jun 1, 2022
Merged

Fix VTableCallHolder writeable mapping size with W^X#70093
janvorli merged 1 commit into
dotnet:mainfrom
janvorli:fix-vtable-stub-holder-mapping-size

Conversation

@janvorli
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

janvorli commented Jun 1, 2022

/backport to release/7.0-preview5

@github-actions
Copy link
Copy Markdown
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