Skip to content

Fixed some potential integer overflows in coreclr#124047

Open
tpa95 wants to merge 9 commits intodotnet:mainfrom
tpa95:fix/coreclr-int-overflow
Open

Fixed some potential integer overflows in coreclr#124047
tpa95 wants to merge 9 commits intodotnet:mainfrom
tpa95:fix/coreclr-int-overflow

Conversation

@tpa95
Copy link
Contributor

@tpa95 tpa95 commented Feb 5, 2026

Fixed some potential integer overflows in gc.cpp and mclist.cpp. The errors were discovered using the Svace static analyzer. I didn't observe any in practice.

In gc_heap::joined_youngest_desired the result of the multiplication will be of type uint32_t, since the highest operand type is uint32_t, so overflow will occur if num_heaps > 255. To promote the result to size_t, you must explicitly convert at least one of the operands to size_t.

In the MCList::processArgAsMCL, an input string consisting of digits and the characters ',' and '-' is parsed into a set of numbers and/or ranges of numbers. If the string contains a number greater than UINT_MAX, the scratch will overflow. The input string often comes from command-line arguments, so protection is necessary.

Found by Linux Verification Center (linuxtesting.org) with SVACE.
Reporter: Pavel Tikhomirov (Tihomirov-P@gaz-is.ru).
Organization: Gazinformservice (resp@gaz-is.ru).

tpa95 added 3 commits February 4, 2026 12:14
In the MCList::processArgAsMCL function, an input string consisting of digits and the characters ',' and '-' is parsed into a set of numbers and/or ranges of numbers. If the string contains a number greater than UINT_MAX, the scratch variable will overflow. The input string often comes from command-line arguments, so protection is necessary.
The result of the multiplication will be of type uint32_t, since the highest operand type is uint32_t, so overflow will occur if num_heaps > 255. To promote the result to size_t, you must explicitly convert at least one of the operands to size_t.
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 5, 2026
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 5, 2026
Comment on lines +42 to +48
unsigned digit = (*head) - '0';
if (scratch > (UINT_MAX - digit) / 10)
{
LogError("Invalid int value in '%s'", input);
return false;
}
scratch = (scratch * 10) + digit;
Copy link
Member

Choose a reason for hiding this comment

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

This will be superseded by #121367. Superpmi is a development-only tool and doesn't need to be robust for invalid inputs.

Copy link
Member

Choose a reason for hiding this comment

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

@tpa95 Can you please address the comment here? This shouldn't be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@huoyaoyuan Sorry, I didn't immediately realize you were the owner and what you meant.
Do you want to remove only the first edit, or both?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not really the owner but contributing to it. All changes in superpmi should be avoided since it's on the way of being refactored.


size_t total_new_allocation = new_allocation * num_heaps;
size_t total_min_allocation = MIN_YOUNGEST_GEN_DESIRED * num_heaps;
size_t total_min_allocation = (size_t)MIN_YOUNGEST_GEN_DESIRED * num_heaps;
Copy link
Member

Choose a reason for hiding this comment

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

The maximum of this product is 16M(MIN_YOUNGEST_GEN_DESIRED) * 1024 (MAX_SUPPORTED_CPUS) = 16G. The multiplication can overflow when >128 heaps are used.

@jkotas jkotas added area-GC-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 5, 2026
@dotnet-policy-service
Copy link
Contributor

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

@tpa95
Copy link
Contributor Author

tpa95 commented Feb 16, 2026

Hi @agocke @dotnet/gc. Could you please take a look when you have a moment and let me know if this is ready to merge or if you’d like changes? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-GC-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants