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

Rethink and/or document our handling of atomic byte slices #77

Open
smalis-msft opened this issue Oct 14, 2024 · 0 comments
Open

Rethink and/or document our handling of atomic byte slices #77

smalis-msft opened this issue Oct 14, 2024 · 0 comments
Assignees
Labels
documentation Improvements or additions to documentation unsafe Related to unsafe code

Comments

@smalis-msft
Copy link
Contributor

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

unsafe { core::ptr::copy_nonoverlapping(self.as_ptr().cast::<u8>(), dest, len) }

safeatomic::atomic_write_ptr - casts self (an &[AtomicU8]) to *mut u8 before passing to copy_nonoverlapping

unsafe { core::ptr::copy_nonoverlapping(src, self.as_ptr() as *mut u8, len) }

guestmem::read_to_atomic - casts an &[AtomicU8] to *mut u8 before passing to try_copy

|| unsafe { self.read_ptr(gpa, dest.as_ptr() as *mut u8, dest.len()) },

guestmem::write_from_atomic - casts an &[AtomicU8] to *const u8 before passing to try_copy

unsafe { self.write_ptr(gpa, src.as_ptr().cast(), src.len()) }

sparse_mmap::atomic_slice - allows getting an &[AtomicU8] from &self, but also allows reading and writing non-atomically through &self

unsafe { std::slice::from_raw_parts((self.as_ptr() as *const AtomicU8).add(start), len) }

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

rust-lang/rust#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.

@smalis-msft smalis-msft added documentation Improvements or additions to documentation unsafe Related to unsafe code labels Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation unsafe Related to unsafe code
Projects
None yet
Development

No branches or pull requests

2 participants