shared memory: audit safety of pointer copies in the presence of shared memories #4203
Labels
wasm-proposal:threads
Issues related to the WebAssembly threads proposal
Comments
abrown
changed the title
alexcrichton
added
the
wasm-proposal:threads
|
For reference, see several SpiderMonkey snippets. |
abrown
added a commit
to abrown/wasmtime
that referenced
this issue
Nov 9, 2022
alexcrichton
pushed a commit
that referenced
this issue
Nov 10, 2022
* wiggle: adapt Wiggle guest slices for `unsafe` shared use When multiple threads can concurrently modify a WebAssembly shared memory, the underlying data for a Wiggle `GuestSlice` and `GuestSliceMut` could change due to access from other threads. This breaks Rust guarantees when `&[T]` and `&mut [T]` slices are handed out. This change modifies `GuestPtr` to make `as_slice` and `as_slice_mut` return an `Option` which is `None` when the underlying WebAssembly memory is shared. But WASI implementations still need access to the underlying WebAssembly memory, both to read to it and write from it. This change adds new APIs: - `GuestPtr::to_vec` copies the bytes from WebAssembly memory (from which we can safely take a `&[T]`) - `GuestPtr::as_unsafe_slice_mut` returns a wrapper `struct` from which we can `unsafe`-ly return a mutable slice (users must accept the unsafety of concurrently modifying a `&mut [T]`) This approach allows us to maintain Wiggle's borrow-checking infrastructure, which enforces the guarantee that Wiggle will not modify overlapping regions, e.g. This is important because the underlying system calls may expect this. Though other threads may modify the same underlying region, this is impossible to prevent; at least Wiggle will not be able to do so. Finally, the changes to Wiggle's API are propagated to all WASI implementations in Wasmtime. For now, code locations that attempt to get a guest slice will panic if the underlying memory is shared. Note that Wiggle is not enabled for shared memory (that will come later in something like #5054), but when it is, these panics will be clear indicators of locations that must be re-implemented in a thread-safe way. * review: remove double cast * review: refactor to include more logic in 'UnsafeGuestSlice' * review: add reference to #4203 * review: link all thread-safe WASI fixups to #5235 * fix: consume 'UnsafeGuestSlice' during conversion to safe versions * review: remove 'as_slice' and 'as_slice_mut' * review: use 'as_unsafe_slice_mut' in 'to_vec' * review: add `UnsafeBorrowResult`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In discussions about #4187, we questioned whether several uses of (the equivalent of)
memsetandmemcpyare still safe when shared memory is present. This issue is a placeholder to discuss that and will be used for tagging the relevant locations in the code.The text was updated successfully, but these errors were encountered: