try_transmute_mut, TryFromBytes::*mut* soundness issue

[zeling]

Progress

• Add Src: FromBytes bound to try_transmute_mut!
• Release new crate version (0.8.16)
• Add Self: IntoBytes bound to TryFromBytes::*mut*
  • For why this is required, consider that a MaybeUninit<u8> is TryFromBytes, but permits writing uninitialized bytes that would invalidated the shadowed src reference.
• Release new crate version (0.8.18)
• Yank old affected versions
• Do deeper surgery to make try_cast_or_pme sound and TryFromBytes::*mut* sound
  • Overhaul Ptr's validity invariant modeling #1866

Original text

Using zerocopy 0.8.13:

use zerocopy::{TryFromBytes, IntoBytes, KnownLayout, Immutable, try_transmute_mut};

#[derive(TryFromBytes, IntoBytes, KnownLayout, Immutable)]
struct T {
    f: bool,
}

fn main() {
    let mut t = T { f: false };
    let slice: &mut [u8; 1] = try_transmute_mut!(&mut t).unwrap();

    slice[0] = u8::MAX;
    println!("f: {}", t.f);
}

cargo +nightly miri run caught an UB:

error: Undefined Behavior: constructing invalid value: encountered 0xff, but expected a boolean
    --> /home/zeling/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2682:25
     |
2682 |         Display::fmt(if *self { "true" } else { "false" }, f)
     |                         ^^^^^ constructing invalid value: encountered 0xff, but expected a boolean
     |
     = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
     = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
     = note: BACKTRACE:
....
     |
13   |     println!("f: {}", t.f);
     |     ^^^^^^^^^^^^^^^^^^^^^^
     = note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

try_transmute_mut may not be a safe API that can be exposed because it only does validation when creating the reference but the user is free to write whatever bit pattern to the created reference.

[jswrenn]

Ack; thanks for the report. It looks like we're missing a FromBytes bound on the source. I'll have a fix up this evening.

[joshlf]

#2229, which fixes this issue at a surface level (it's sound, but doesn't fix the underlying internals that allowed this to slip through) is published in 0.8.16.

[jswrenn]

The underlying issue that led to us not enforce a FromBytes bound on try_transmute_mut also led us to omit a Self: IntoBytes bound on TryFromBytes's mut methods. Our intention is to release a surface-level fix in 0.8, and then overhaul Ptr in 0.9 to make such mistakes less likely (see #1866).