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 memory unsafety when array offset values are >isize::MAX #34

Open
jdm opened this issue Oct 3, 2016 · 3 comments
Open

Prevent memory unsafety when array offset values are >isize::MAX #34

jdm opened this issue Oct 3, 2016 · 3 comments

Comments

@jdm
Copy link
Member

jdm commented Oct 3, 2016

From #28:

All of these casts to isize when offsetting are unsafe, since they could become negative numbers and cause us to write outside of the bounds of the array. We should either use to_isize.unwrap() and panic if that occurs, or use a conversion strategy that yields a value that will gives us worse performance but correct behaviour.
At a quick readthrough, it looks like push, pop, truncate, remove, insert all suffer from the same issue if len > isize::MAX.

It looks like one might be able to manipulate that situation in a call to insert() when len == isize::MAX (then the len would get set to size::MAX + 1).
@emilio
Copy link
Member

emilio commented Oct 4, 2016

I doubt it's worth to fix that if this makes SmallVec slower. Any array of isize::MAX elements is literally half of the address space in the computer.

@mbrubeck
Copy link
Collaborator

It's currently impossible to create a SmallVec with a length greater than isize::MAX, because Vec::with_capacity will panic if the capacity overflows isize. I'm not sure if this is guaranteed to remain true in future versions of Rust, though. (It's not documented in the public libstd docs, though it is documented in the unstable liballoc documentation.)

@mbrubeck
Copy link
Collaborator

(We could remove all doubt by adding our own assertions that the capacity never exceeds isize::MAX.)

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

No branches or pull requests

3 participants