Skip to content

Fix unsoundness in FromBytes::new_box_slice_zeroed #63

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Oct 15, 2022

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented Oct 15, 2022

Previously, FromBytes::new_box_slice_zeroed called Layout::from_size_align_unchecked with arguments that could overflow isize, which would violate its safety preconditions. In this change, we use the safe variant, which returns a Result which we can .expect(). Though I haven't benchmarked it, this likely has little impact on performance over the optimal code since optimal code would still need to perform the same bounds check that from_size_align is performing.

Previously, `FromBytes::new_box_slice_zeroed` called
`Layout::from_size_align_unchecked` with arguments that could overflow
`isize`, which would violate its safety preconditions. In this change,
we use the safe variant, which returns a `Result` which we can
`.expect()`. Though I haven't benchmarked it, this likely has little
impact on performance over the optimal code since optimal code would
still need to perform the same bounds check that `from_size_align` is
performing.
@joshlf joshlf force-pushed the new_box_slice_zeroed-unsound branch from 2da84e5 to 4b5efec Compare October 15, 2022 04:28
@joshlf joshlf merged commit 7a1617a into main Oct 15, 2022
@joshlf joshlf deleted the new_box_slice_zeroed-unsound branch October 15, 2022 04:37
@joshlf
Copy link
Member Author

joshlf commented Oct 15, 2022

Breadcrumbs: Looks like the test failure in this PR is related to rust-lang/rust#95295 and rust-lang/rust#101899.

joshlf added a commit that referenced this pull request Aug 3, 2023
Previously, `FromBytes::new_box_slice_zeroed` called
`Layout::from_size_align_unchecked` with arguments that could overflow
`isize`, which would violate its safety preconditions. In this change,
we use the safe variant, which returns a `Result` which we can
`.expect()`. Though I haven't benchmarked it, this likely has little
impact on performance over the optimal code since optimal code would
still need to perform the same bounds check that `from_size_align` is
performing.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant