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 commit accounting for large pages #78531

Merged
merged 1 commit into from
Nov 18, 2022

Conversation

cshung
Copy link
Member

@cshung cshung commented Nov 18, 2022

This change fixes an integer underflow that happens in the commit accounting, here is the sequence of events that will lead to it.

Under use_large_page_p, we created a new heap segment, we passed SEGMENT_INITIAL_COMMIT (which is just a page) to virtual_commit, so committed_by_oh[0] is increased by SEGMENT_INITIAL_COMMIT.

Later, the heap_segment_committed value for the segment is changed to size, which is the full region size.

Eventually, we return_free_region, which will subtract committed_by_oh[0] by the full region size, which will lead to a subtraction integer underflow.

This will result in a huge number, and that will fail future virtual_commit calls, leading to unexpected OOM.

The fix simply makes sure we add the full region size to the committed_by_oh[0] to begin with.

@cshung cshung requested a review from mrsharm November 18, 2022 01:24
@cshung cshung self-assigned this Nov 18, 2022
@ghost
Copy link

ghost commented Nov 18, 2022

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

This change fixes an integer underflow that happens in the commit accounting, here is the sequence of events that will lead to it.

Under use_large_page_p, we created a new heap segment, we passed SEGMENT_INITIAL_COMMIT (which is just a page) to virtual_commit, so committed_by_oh[0] is increased by SEGMENT_INITIAL_COMMIT.

Later, the heap_segment_committed value for the segment is changed to size, which is the full region size.

Eventually, we return_free_region, which will subtract committed_by_oh[0] by the full region size, which will lead to a subtraction integer underflow.

This will result in a huge number, and that will fail future virtual_commit calls, leading to unexpected OOM.

The fix simply makes sure we add the full region size to the committed_by_oh[0] to begin with.

Author: cshung
Assignees: cshung
Labels:

area-GC-coreclr

Milestone: -

@cshung cshung force-pushed the public/fix-commit-accounting branch from df7676d to aec1429 Compare November 18, 2022 01:31
@cshung cshung force-pushed the public/fix-commit-accounting branch from aec1429 to a86c1ac Compare November 18, 2022 02:07
@cshung cshung merged commit 793676e into dotnet:main Nov 18, 2022
@cshung cshung deleted the public/fix-commit-accounting branch November 18, 2022 16:08
@cshung
Copy link
Member Author

cshung commented Nov 18, 2022

/backport to release/7.0

@github-actions
Copy link
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3498854226

@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 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