tock / libtock-rs

Rust userland library for Tock
Apache License 2.0
168 stars 109 forks source link

Update rust-toolchain to nightly of 2023-07-30 #481

Closed lschuermann closed 1 year ago

lschuermann commented 1 year ago

This version and the time to update is chosen somewhat arbitrarily, as libtock-rs fails to build elf2tab on its current Rust toolchain (2022-06-10).

lschuermann commented 1 year ago

Okay, I think I hit a roadblock. After the update, Miri fails with the following:

error: unsupported operation: integer-to-pointer casts and `ptr::from_exposed_addr` are not supported with `-Zmiri-strict-provenance`
   --> /home/runner/work/libtock-rs/libtock-rs/platform/src/register.rs:25:18
    |
25  |         Register(value as *mut ())
    |                  ^^^^^^^^^^^^^^^^ integer-to-pointer casts and `ptr::from_exposed_addr` are not supported with `-Zmiri-strict-provenance`
    |
    = help: use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead
    = note: BACKTRACE:
    = note: inside `<libtock_platform::Register as core::convert::From<u32>>::from` at /home/runner/work/libtock-rs/libtock-rs/platform/src/register.rs:25:18: 25:34
    = note: inside `<u32 as core::convert::Into<libtock_platform::Register>>::into` at /home/runner/.rustup/toolchains/nightly-2023-07-30-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:716:9: 716:22
note: inside `libtock_platform::syscalls_impl::<impl libtock_platform::Syscalls for libtock_unittest::fake::Syscalls>::command`

Having read the linked strict provenance APIs documentation, I can understand why this is an error. However, I don't really know how to work around it. As far as I see it, we're using the Register in raw_syscalls as a means to provide Miri with the ability to track pointers as they are passed across the fake system call interface boundaries. By using raw pointers instead of usize integers, those pointers can carry an additional tag which Miri uses for its provenance checks. This is not actually used at runtime in a compiled Rust program, as carrying provenance across the asynchronous system call boundary (such as in the case of Tock's upcalls) wouldn't work anyways -- we're fundamentally limited to a usize-sized type here. So far, so good.

The root cause of the issues we're facing here is that we can't create a raw pointer type from a "tagless" usize integer. Semantically, accessing such a pointer does not make sense with provenance analysis. Miri can't know that we're using this pointer solely as a usize-sized (which is actually not necessarily true on all platforms, such as Miri) type to hold an integer in its address field, only to extract this integer back later. We never actually dereference this pointer.

From what I understand, Miri doesn't leave us with too many options here, assuming we want to stick to using strict provenance checks:

I'll prototype the second approach for now, and will see whether I can work around the Miri issues this way.

lschuermann commented 1 year ago

My "fix" seems to work, but it does prevent compilation on stable Rust. As far as I know, the only way to have this work on both stable (for compiling) and unstable (for testing with Miri) is to use conditional compilation. If we need to use conditional compilation anyways, it might make sense to go with the enum-route instead for Miri builds, and use the previous raw pointer-based approach for everything else.

jrvanwhy commented 1 year ago

I wouldn't rule out the possibility of this code running on a platform where size_of::<*mut ()> != size_of::<usize>().

I think the most standard/idiomatic fix here is to use sptr::invalid. sptr exists primarily to allow Rust code to work on both stable and under strict provenance rules, which is the problem we're facing.

If you really want to avoid adding another dependency, you could try copying the implementation from sptr, which is just a transmute:

    // FIXME(strict_provenance_magic): I am magic and should be a compiler intrinsic.
    // We use transmute rather than a cast so tools like Miri can tell that this
    // is *not* the same as from_exposed_addr.
    // SAFETY: every valid integer is also a valid pointer (as long as you don't dereference that
    // pointer).
    #[cfg(miri)]
    return unsafe { core::mem::transmute(addr) };

Note: we don't need conditional compilation here because we don't need const.

However, I'm not 100% sure that'll work, because it's possible that Miri special-cases the sptr crate.

lschuermann commented 1 year ago

Closing in favor of #511, thanks @jrvanwhy!