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

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

Closed
clarfonthey opened this issue Aug 13, 2024 · 4 comments

Comments

@clarfonthey
Copy link

clarfonthey commented Aug 13, 2024

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
#[repr(transparent)]
pub struct ForwardInit<T> {
    inner: MaybeUninit<T>,
}
impl<T> ForwardInit<T> {
    pub fn init(&mut self, value: T) -> &mut T {
        self.inner.write(value)
    }
    pub fn from_maybe_uninit_mut(maybe: &mut MaybeUninit<T>) -> &mut ForwardInit<T> {
        // SAFETY: By `repr(transparent)`, these have the same layout.
        unsafe { &mut *ptr::from_mut(maybe).cast::<ForwardInit<T>>() }
    }
    pub fn from_mut(value: &mut T) -> &mut ForwardInit<T> {
        // SAFETY: By `repr(transparent)`, these have the same layout.
        unsafe { &mut *ptr::from_mut(value).cast::<ForwardInit<T>>() }
    }
}

#[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
Copy link
Contributor

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

  • The new mutable reference after retagging can't tag every byte it would refer to (IE. because it falls outside of the allocation, or lacking permission, under SB it would also be if the new referenced type is larger than the previous one)
  • The new reference is unaligned for the type of the reference (For example, transmuting &mut [u8;4] to &mut u32 would be UB if the reference wasn't 4-aligned).

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

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

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
Copy link
Author

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
Copy link
Member

RalfJung commented Aug 13, 2024

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 #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 #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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants