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

Prevent allocator underflow #248

Merged
merged 1 commit into from
Oct 14, 2021
Merged

Prevent allocator underflow #248

merged 1 commit into from
Oct 14, 2021

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented Oct 14, 2021

Goals

Address scenario that could cause an underflow in the allocator (leading to total allocated
being very high)

The scenario is:

  • peer 1 allocates block amount x
  • peer 2 allocates block amount y >= 1
  • peer 1 call to deallocate block amount x+1 (this shouldn't happen somehow it does). We protect against peer 1 allocation going below zero by setting it to 0, but the global total is now : y-1 (x+y - (x+1))
  • peer 2 deallocates entirely. total is now -1 (no check of total allocated >= 0 after peer deallocation). on unsigned int this becomes MaxUint64

Implementation

  • Add test to demonstrate issues
  • Add precautionary check on peer deallocations to prevent underflow
  • When deallocating blocks that are greater than total peer memory, only subtract total peer memory from global total, since that's what is actually deallocated
  • Add logging to all these unusual scenarios to try to understand how often they are happening

address scenario that could cause a buffer overflow in the allocator (leading to total allocated
being very high)
@hannahhoward hannahhoward merged commit 4060815 into main Oct 14, 2021
@rvagg
Copy link
Member

rvagg commented Oct 15, 2021

peer 1 call to deallocate block amount x+1 (this shouldn't happen somehow it does).

So we still have a persistent bug in here somewhere and should be on the lookout for causes? Are we confident this is happening from the data we're getting from live use?

@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
@mvdan mvdan deleted the fix/allocator-overflow branch December 15, 2021 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants