microsoft / microsoft/openvmm

Rethink and/or document our handling of atomic byte slices

Open
#77 0 comments 0 reactions 1 assignee View on GitHub

@jstarks is already working on this.

Since Oct 14, 2024.

documentation unsafe
Dominant language
Rust
Stars
1.9k
Forks
238
Avg merge
1d 15h
Merged PRs (30d)
100

Description

The assumption that it is safe to perform wider-than-size non-atomic reads and writes on a &[AtomicFoo] is rather deeply baked into our codebase. However, under the current Rust safety rules this is UB, as it exposes the potential to observe data races. In practice it appears to work fine, but it's definitely not a good look, and who knows if it will continue to work fine. We should rethink how we handle these cases, or at least add some expansive docs on why we're doing what we're doing and any precautions we take. Some examples:

safeatomic::atomic_read_ptr - casts self (an &[AtomicU8]) to *const u8 before passing to copy_nonoverlapping
https://github.com/microsoft/openvmm/blob/aedf1e8bda810a21bc8435554c7dd1fd7ea32917/support/safeatomic/src/lib.rs#L124

safeatomic::atomic_write_ptr - casts self (an &[AtomicU8]) to *mut u8 before passing to copy_nonoverlapping
https://github.com/microsoft/openvmm/blob/aedf1e8bda810a21bc8435554c7dd1fd7ea32917/support/safeatomic/src/lib.rs#L137

guestmem::read_to_atomic - casts an &[AtomicU8] to *mut u8 before passing to try_copy
https://github.com/microsoft/openvmm/blob/aedf1e8bda810a21bc8435554c7dd1fd7ea32917/vm/vmcore/guestmem/src/lib.rs#L1343

guestmem::write_from_atomic - casts an &[AtomicU8] to *const u8 before passing to try_copy
https://github.com/microsoft/openvmm/blob/aedf1e8bda810a21bc8435554c7dd1fd7ea32917/vm/vmcore/guestmem/src/lib.rs#L1262

sparse_mmap::atomic_slice - allows getting an &[AtomicU8] from &self, but also allows reading and writing non-atomically through &self
https://github.com/microsoft/openvmm/blob/aedf1e8bda810a21bc8435554c7dd1fd7ea32917/support/sparse_mmap/src/lib.rs#L574

There may be other cases I'm not aware of too, this is not necessarily an exhaustive list.

https://github.com/rust-lang/rust/pull/128778 will allow mixed-size reads, and allow racing non-atomic and atomic operations, which definitely helps us a lot. It does not allow for racing mixed-size non-atomic writes with other atomic operations though.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.