rust-lang / unsafe-code-guidelines

Forum for discussion about what unsafe code can and can't do
https://rust-lang.github.io/unsafe-code-guidelines
Apache License 2.0
657 stars 57 forks source link

Is it UB to have a mutable reference that could be misused, or simply to misuse it? #524

Closed clarfonthey closed 1 month ago

clarfonthey commented 1 month ago

Essentially, both the reference and MIRI seem to be okay with this, but I don't think this has been explicitly specified, and it probably should be.

Clarifying the question:

  1. Given a value x of arbitrary type T
  2. Given a type U with the same layout as T
  3. Assuming that the current value of x is valid for both T and U
  4. Is it UB to cast &mut x to &mut U?

A few examples for T and U might be char and u32, or more generally T and MaybeUninit<T>, the latter being the case I specifically am interested in.

Essentially, we know that in all cases, if you actually assign a value to x via &mut U that isn't valid for T, that is definitely UB. For example, if T is char, then it would be invalid to cast the reference of &mut x to &mut u32 and then assign it the value of u32::MAX, for example.


For a bit of background: I would like to generalise code such that it can accept either &mut T or &mut MaybeUninit<T>, and ideally, I would be able to cast both of these types to a third type, &mut ForwardInit<T>, which allows writing values of T but does not allow writing values of MaybeUninit<T>. This would presumably be sound, since we can't write an uninit value back into &mut T.

However, the issue here is that we would have to define ForwardInit<T> like:

#[repr(transparent)]
struct ForwardInit<T> {
    inner: MaybeUninit<T>,
}

and, indirectly, we would be casting our &mut T to &mut MaybeUninit<T>. However, we can still technically keep this sound if we ensure that, via our API for ForwardInit<T>, we only allow writing initialized values, and disallow writing uninitialized values.

This is kind of similar to Exclusive<T> in spirit, since all we're doing is changing the API for the type, without changing anything inside the type. Except in this case, we're actually changing the inside of the type, but making sure that the API on the surface can't actually take advantage of this to do an invalid write. Assuming that it's the write that is UB, and not the reference itself.


Here's the code I used to test whether MIRI was okay with this kind of casting:

See the code ```rust #[repr(transparent)] pub struct ForwardInit { inner: MaybeUninit, } impl ForwardInit { pub fn init(&mut self, value: T) -> &mut T { self.inner.write(value) } pub fn from_maybe_uninit_mut(maybe: &mut MaybeUninit) -> &mut ForwardInit { // SAFETY: By `repr(transparent)`, these have the same layout. unsafe { &mut *ptr::from_mut(maybe).cast::>() } } pub fn from_mut(value: &mut T) -> &mut ForwardInit { // SAFETY: By `repr(transparent)`, these have the same layout. unsafe { &mut *ptr::from_mut(value).cast::>() } } } #[cfg(test)] mod tests { use core::mem::MaybeUninit; use crate::ForwardInit; #[test] fn init_maybe_uninit_int() { let mut maybe = MaybeUninit::uninit(); let fwd = ForwardInit::from_maybe_uninit_mut(&mut maybe); fwd.init(1); assert_eq!(unsafe { maybe.assume_init() }, 1); } #[test] fn init_init_int() { let mut init = 4; let fwd = ForwardInit::from_mut(&mut init); fwd.init(1); assert_eq!(init, 1); } #[test] fn init_maybe_uninit_char() { let mut maybe = MaybeUninit::uninit(); let fwd = ForwardInit::from_maybe_uninit_mut(&mut maybe); fwd.init('あ'); assert_eq!(unsafe { maybe.assume_init() }, 'あ'); } #[test] fn init_init_char() { let mut init = 'A'; let fwd = ForwardInit::from_mut(&mut init); fwd.init('あ'); assert_eq!(init, 'あ'); } } ```
chorman0773 commented 1 month ago

A type changing transmute for a mutable reference is undefined behaviour in the following cases:

The following is currently an open question whether it would be UB:

It's very much legal to transmute between any kind of reference even if you could "misuse" the reference to write an invalid value. Rust memory isn't typed, and a reference to some memory doesn't know what "type" that memory might be used as until the actual use. In fact, it's not inherently UB to, say, transmute a &mut char to a &mut u32 and write 0x110000 to it. Exactly when it becomes UB is still up for debate on several issues, but the transmute itself is definitely fine while the former reference is still valid (IE. hasn't been disabled by a foreign read/write).

clarfonthey commented 1 month ago

Hmm, it's interesting that you say that writing 0x11000 isn't necessarily UB in that case; I guess that if the &mut char actually referred to a value that was originally a u32, that wouldn't be invalid.

I also when looking more into this noticed that BorrowedBuf currently relies on this behaviour too, so, that's good to know.

RalfJung commented 1 month ago

The following is currently an open question whether it would be UB:

  • If the referent is an invalid value of the referenced type.

Indeed, that's https://github.com/rust-lang/unsafe-code-guidelines/issues/412.

In fact, it's not inherently UB to, say, transmute a &mut char to a &mut u32 and write 0x110000 to it. Exactly when it becomes UB is still up for debate on several issues

And that's https://github.com/rust-lang/unsafe-code-guidelines/issues/84.

I think those two issues cover all questions you raise? It is definitely allowed to transmute an &mut char to an &mut u32 and then only ever write valid char into that; given the Rust's memory is untyped, there's no way we'd ever even notice that something is odd here. (Of course this assumes the aliasing requirements for &mut are upheld.)

clarfonthey commented 1 month ago

Indeed, I think that my questions are covered by those two; I'll close this. Thank you both!