Validation: check raw wide pointer metadata

[RalfJung]

While I was at it, I also added a missing check for slices not to be too big.

r? @oli-obk
Fixes rust-lang/miri#918

[oli-obk]

So https://doc.rust-lang.org/std/ptr/fn.slice_from_raw_parts.html is wrong? It's a safe function allowing the construction of such invalid slices.

It can also be done safely on stable:

123456789 as *const [(); usize::max_value()] as *const [()] as *const [u64]

Did anything change in that regard?

[RalfJung]

Ouch, I forgot about that.

What changed is that when slice_from_raw_parts landed, nobody was aware of #62603 #63851.

[RalfJung]

Looks like @eddyb was the only one who even knew that we require wide raw ptr metadata to be valid.^^ Nobody in #60667 and rust-lang/unsafe-code-guidelines#166 knew.

[RalfJung]

For this PR, we could (for now) say the fact that this cast is allowed is a bug, and wait until #63851 / rust-lang/unsafe-code-guidelines#166 are resolved. That would match the UB lists in the Nomicon / Reference.

Or we do the slice length check not for raw pointers. That would make the UB documentation more messy. Or we declare the slice length to be just part of the safety invariant of slices so that there is nothing to check, keeping UB simpler.

[eddyb]

(offtopic) @oli-obk why doesn't TyCtxtAt just implement the right traits to not require all of these &* in miri?

[oli-obk]

slice_from_raw_parts is unstable, so it's not an issue. But the stable transformation is indeed problematic.

What changed is that when slice_from_raw_parts landed, nobody was aware of #62603.

what does #62603 have to do with raw pointer metadata?

[RalfJung]

@oli-obk I removed the new slice length check.

[RalfJung]

what does #62603 have to do with raw pointer metadata?

Sorry, copy-pasted the wrong thing. I meant #63851.

[oli-obk]

@bors r+

[bors]

📌 Commit 04580b6 has been approved by oli-obk

[RalfJung]

@bors r-

I also found where the slice size check actually happens, and added a comment. @oli-obk does that seem okay? Or should I try to fix the ICE here as well?

[RalfJung]

Hm actually I cannot get it to ICE...

use std::slice;
use std::usize;

fn main() { unsafe {
    let ptr = Box::into_raw(Box::new(0u8));
    let _x = slice::from_raw_parts(ptr, usize::MAX);
} }

[RalfJung]

Oh wow, that's odd. When multiplying Size with u64, a check occurs, but not when multiplying Size with Size`?

rust/src/librustc_target/abi/mod.rs

Lines 341 to 360 in 7e0afda

	impl Mul<Size> for u64 {
	type Output = Size;
	#[inline]
	fn mul(self, size: Size) -> Size {
	size * self
	}
	}
	impl Mul<u64> for Size {
	type Output = Size;
	#[inline]
	fn mul(self, count: u64) -> Size {
	match self.bytes().checked_mul(count) {
	Some(bytes) => Size::from_bytes(bytes),
	None => {
	panic!("Size::mul: {} * {} doesn't fit in u64", self.bytes(), count)
	}
	}
	}
	}

@eddyb is that deliberate?

EDIT: Never mind, that's u64 * Size and it just dispatches to the other impl.

[RalfJung]

Oh, Size multiplication actually is not doing the right kind of check for us here... it calls checked_mul for u64, not for Size.

I will remove the last commit here and make that a new PR.

[RalfJung]

@bors r=oli-obk

[bors]

📌 Commit 04580b6 has been approved by oli-obk

[bors]

⌛ Testing commit 04580b6 with merge 7445622...

[bors]

☔ The latest upstream changes (presumably #63998) made this pull request unmergeable. Please resolve the merge conflicts.

[pietroalbini]

@bors r- retry