Check page capacity before obtaining base pointer #4731
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This doesn't cause any issues in practice because this is a private API that is only used in ways that cannot trigger UB. Indexing into `slots` is not sound until after we've asserted that the page is allocated, since that aliases the first slot which may not be allocated. This PR also switches to using `as_ptr` to obtain the base pointer for clarity.
djkoloski
force-pushed
the
harden_slab_index_for
branch
from
2881af5
to
f6d2f80
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This doesn't cause any issues in practice because this is a private API that is only used in ways that cannot trigger UB. Indexing into
slotsis not sound until after we've asserted that the page is allocated, since that aliases the first slot which may not be allocated. This PR also switches to usingas_ptrto obtain the base pointer for clarity.Motivation
While reviewing an upgrade of tokio for Fuchsia, we identified a potential soundness issue in
Slots::index_for. In the future, changes to the uses of the structure could cause UB.Solution
The assertion that the page is not empty has been moved earlier in the function and the method for getting a pointer to the first
Vecelement has been changed from&slots[0] as *const _toslots.as_ptr()which was stabilized in 1.37 and meets MSRV.