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

Not necessarily safe #1

Closed
BurntSushi opened this issue Nov 6, 2016 · 8 comments
Closed

Not necessarily safe #1

BurntSushi opened this issue Nov 6, 2016 · 8 comments
Labels

Comments

@BurntSushi
Copy link

BurntSushi commented Nov 6, 2016

Unfortunately, the API exposed here is most definitely not safe. This program (on x86_64) produces a seg fault, yet there is no unsafe. Therefore, guarded_transmute needs to be labeled as an unsafe function.

extern crate safe_transmute;

use safe_transmute::guarded_transmute;

fn main() {
    let x: &Vec<i32> = guarded_transmute(&[
        0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
    ]);
    println!("{:?}", x);
}
@nabijaczleweli
Copy link
Owner

Well, it's as safe as the data you feed to it and that looks like you're making a null pointer

@nabijaczleweli nabijaczleweli changed the title not safe Not necessarily safe Nov 6, 2016
@nabijaczleweli
Copy link
Owner

Also, there's no mention of safety anywhere in the docs, just "checked" and "guarded".

@BurntSushi
Copy link
Author

Also, there's no mention of safety anywhere in the docs

You've defined guarded_transmute as a safe function by virtue of omitting unsafe.

Well, it's as safe as the data you feed to it and that looks like you're making a null pointer

... which should be impossible in safe code. If I can use your API to dereference a null pointer without ever using unsafe, then your API isn't safe and therefore must be marked as unsafe.

@nabijaczleweli
Copy link
Owner

See updated documentation in referenced commit

@BurntSushi
Copy link
Author

I'm afraid the updated documentation is incorrect. You can read about what it means to be unsafe in Rust here. unsafe has a very specific meaning, and it's clearly being violated here. If you declare a function as safe (which you've done here, because unsafe is omitted in the function declaration) then it must be safe for all inputs.

@nabijaczleweli
Copy link
Owner

nabijaczleweli commented Nov 6, 2016

Good point, so to-do list:

  • Dereferencing a NULL/dangling raw pointer - not done in library functions, so we're good
  • Invalid values in primitive types, even in private fields/locals:
    • NULL/dangling references or boxes - unfortunately possible, we should deny &T

@nabijaczleweli nabijaczleweli reopened this Nov 6, 2016
@BurntSushi
Copy link
Author

BurntSushi commented Nov 6, 2016

You can't just deny &T though. e.g.,

#[derive(Clone, Copy)]
struct Foo {
    x: &'static i32,
}

Now I can do let x: Foo = guarded_transmute(&[...]) and wind up with the same problem.

And I'm pretty sure your problems aren't limited to pointers either. What about this?

#[derive(Clone, Copy)]
enum Foo {
   X(i32),
   Y(i32),
}

Then you could do let foo: Foo = guarded_transmute(&[...]) and wind up with an invalid tag in the value.

I bet there are more problems too.

I think your idea with adding extra checks is a good thing, but I don't really see how you can make an API like this generally safe.

See also: https://doc.rust-lang.org/nomicon/transmutes.html

@nabijaczleweli
Copy link
Owner

You're right :v

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

No branches or pull requests

2 participants