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 potential buffer overflow in insert_many #254

Merged
merged 1 commit into from Jan 8, 2021

Conversation

mbrubeck
Copy link
Contributor

@mbrubeck mbrubeck commented Jan 8, 2021

Fixes #252.

@mbrubeck
Copy link
Contributor Author

mbrubeck commented Jan 8, 2021

Pushed a different version of the fix. This is a more invasive change, but it leaves the code a bit simpler and much easier to reason about.

The key observation is that the guard is no longer needed after we fill up the initial lower_size_bound slots of reserved space. While looping through any surplus items from the iterator, the SmallVec is in a valid state at the start and end of each iteration.

emilio
emilio approved these changes Jan 8, 2021
Copy link
Member

@emilio emilio left a comment

r=me with that change unless I'm missing something.

src/lib.rs Outdated Show resolved Hide resolved
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Jan 8, 2021

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2021

📌 Commit 9998ba0 has been approved by emilio

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2021

Testing commit 9998ba0 with merge 4e53e07...

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2021

☀️ Test successful - checks-travis
Approved by: emilio
Pushing 4e53e07 to master...

@bors-servo bors-servo merged commit 4e53e07 into servo:master Jan 8, 2021
2 checks passed
mbrubeck added a commit that referenced this issue Jan 8, 2021
Backport of #254 to the 0.6 branch.  Fixes #253.
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