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 WasmPtr to work with accesses accessing the final valid byte #1272

Merged
merged 4 commits into from
Mar 6, 2020

Conversation

MarkMcCaskey
Copy link
Contributor

@MarkMcCaskey MarkMcCaskey commented Mar 5, 2020

Resolves #1258

The fix was to change >= into >. Doing this made us vulnerable to accessing memory to create an empty slice just out of bounds so we have to add some additional checks to make sure that that can't happen.

This PR also prevents accessing arrays of length 0 (the length bound is non-inclusive, so length 0 is never valid) and prevents access of zero-sized types.

The zero-sized type checks will probably be inlined (or will be in the future as const fn gets more mature) so provide no additional overhead. The checking of if length == 0 does add some overhead, but on modern CPUs it shouldn't be an issue as it's a branch that should be always false in normal use.

Review

  • Add a short description of the the change to the CHANGELOG.md file

This also returns `None` for all accesses of zero sized types and
arrays of length 0.  Because the array accesses have a non-inclusive
length, length of 0 is not valid.  These checks prevent returning
empty slices that point just outside of memory bounds.
@MarkMcCaskey MarkMcCaskey added the 📦 lib-deprecated About the deprecated crates label Mar 5, 2020
@MarkMcCaskey MarkMcCaskey requested a review from nlewycky March 5, 2020 21:28
@MarkMcCaskey MarkMcCaskey requested a review from Hywan as a code owner March 5, 2020 21:28
Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests?

@MarkMcCaskey MarkMcCaskey force-pushed the fix/wasmptr-last-byte-access branch from 490c2ac to f8d34e0 Compare March 5, 2020 23:08
@MarkMcCaskey
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 6, 2020

@bors bors bot merged commit 4861e6e into master Mar 6, 2020
@bors bors bot deleted the fix/wasmptr-last-byte-access branch March 6, 2020 00:10
@Hywan
Copy link
Contributor

Hywan commented Mar 6, 2020

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 lib-deprecated About the deprecated crates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WASI fd_write and fd_read fails on memory boundary
3 participants