miri icon indicating copy to clipboard operation
miri copied to clipboard

Integers and pointers sometimes incorrectly preserve provenance on typed copies

Open RalfJung opened this issue 2 years ago • 3 comments

Miri currently does not properly implement what happens to provenance during a typed copy:

  • When doing a copy at integer type, provenance should be stripped. Instead, we will currently complain during validation that provenance on an integer is UB.
  • When doing a copy at pointer type, if the provenance is not the same for all bytes of a pointer, it should be stripped. Instead, we will currently just preserve it.

The best way I see to solve this is to make validation mutating, so that during validation we can adjust integer and pointer values following these rules. Mutating validation is also needed for https://github.com/rust-lang/miri/issues/845.

RalfJung avatar Jun 03 '22 12:06 RalfJung

Testcase for the first problem.

This should fail.

use std::mem;

fn main() {
    let ptrs = [&42];
    let ints: [usize; 1] = unsafe { mem::transmute(ptrs) };
    let ptr = ints.as_ptr().cast::<&i32>();
    let _val = unsafe { *ptr.read() }; //~ERROR
}

Testcase for the second problem.

This should fail.

fn main() { unsafe {
    let mut bytes = [1u8; 16];
    let bytes = bytes.as_mut_ptr();
    
    // Put a pointer in the middle.
    bytes.add(4).cast::<&i32>().write_unaligned(&42);
    // Copy the entire thing as two pointers but not perfectly
    // overlapping with the pointer we have in there.
    let copy = bytes.cast::<[*const (); 2]>().read_unaligned();
    let copy_bytes = copy.as_ptr().cast::<u8>();
    // Now go to the middle of the copy and get the pointer back out.
    let ptr = copy_bytes.add(4).cast::<*const i32>().read_unaligned();
    // Dereferencing this should fail as the copy has removed the provenance.
    let _val = *ptr; //~ERROR
} }

RalfJung avatar Jun 03 '22 14:06 RalfJung

Seems like testcase 1 currently passes but I didn't dig too deep to see why/since when; maybe this issue is worth updating for anyone coming to work on it in the future?

nia-e avatar Dec 11 '22 21:12 nia-e

Ah right that got fixed by https://github.com/rust-lang/rust/pull/104054 (or maybe even earlier).

RalfJung avatar Dec 11 '22 21:12 RalfJung