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

Unsafe trait implementation for IntoBytes is incorrect (and UB) #2472

Open
gendx opened this issue Nov 25, 2024 · 1 comment
Open

Unsafe trait implementation for IntoBytes is incorrect (and UB) #2472

gendx opened this issue Nov 25, 2024 · 1 comment

Comments

@gendx
Copy link
Collaborator

gendx commented Nov 25, 2024

The IntoBytes implementation is incorrect, as shown when running this program under Miri. The correct syntax is &raw const *self (which returns a *const Self) rather than &raw const self (which returns a *const &Self).

use std::{mem, slice};

/// ...
/// # Safety
/// The type must have a defined representation and no padding.
pub unsafe trait IntoBytes {
    fn as_bytes(&self) -> &[u8] {
        let len = mem::size_of_val(self);
        unsafe { slice::from_raw_parts((&raw const self).cast::<u8>(), len) }
    }
}

// SAFETY: `u32` has a defined representation and no padding.
unsafe impl IntoBytes for u32 {}

fn main() {
    println!("{:?}", 42.as_bytes());
}
error: Undefined Behavior: out-of-bounds pointer use: alloc811 has been freed, so this pointer is dangling
  --> src/main.rs:17:22
   |
17 |     println!("{:?}", 42.as_bytes());
   |                      ^^^^^^^^^^^^^ out-of-bounds pointer use: alloc811 has been freed, so this pointer is dangling
   |
   = 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
help: alloc811 was allocated here:
  --> src/main.rs:7:17
   |
7  |     fn as_bytes(&self) -> &[u8] {
   |                 ^^^^^
help: alloc811 was deallocated here:
  --> src/main.rs:10:6
   |
10 |     }
   |      ^
   = note: BACKTRACE (of the first span):
   = note: inside `main` at src/main.rs:17:22: 17:35

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
@djmitche
Copy link
Collaborator

While looking, I notice that while we are now using the &raw syntax, we don't introduce it anywhere!

That slide was updated in b4f07ba, and it looks like that introduced the bug.

Do you want to make a PR to fix this? Bonus points for adding a quick introduction to the &raw syntax!

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

2 participants