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

Miri with strict provenance flags UB in macos impl of hashmap_random_keys #96163

Closed
lopopolo opened this issue Apr 18, 2022 · 4 comments · Fixed by #96167
Closed

Miri with strict provenance flags UB in macos impl of hashmap_random_keys #96163

lopopolo opened this issue Apr 18, 2022 · 4 comments · Fixed by #96167
Labels
A-miri Area: The miri tool C-bug Category: This is a bug.

Comments

@lopopolo
Copy link
Contributor

I tried this code:

ManuallyDrop::new(HashMap::with_capacity(capacity))

https://github.com/artichoke/intaglio/tree/da131444a22fc9b4d9382be5ecb16583cddb31d5

With this miri invocation:

$ MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-check-number-validity -Zmiri-symbolic-alignment-check -Zrandomize-layout" cargo +nightly miri test --test leak_drop

I expected to see this happen: no Miri-flagged UB in std

Instead, this happened:

running 5 tests
test bytes::dealloc_owned_data ... error: Undefined Behavior: 0x1ac318 is not a valid pointer
   --> /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:130:40
    |
130 |                     let ret = unsafe { f(s.as_mut_ptr() as *mut c_void, s.len()) };
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 0x1ac318 is not a valid pointer
    |
    = 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

    = note: inside closure at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:130:40
    = note: inside `std::option::Option::<unsafe extern "C" fn(*mut std::ffi::c_void, usize) -> i32>::map::<bool, [closure@std::sys::unix::rand::imp::getentropy_fill_bytes::{closure#0}]>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/option.rs:909:29
    = note: inside `std::sys::unix::rand::imp::getentropy_fill_bytes` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:125:9
    = note: inside `std::sys::unix::rand::imp::fill_bytes` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:141:12
    = note: inside `std::sys::unix::rand::hashmap_random_keys` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:8:9
    = note: inside `std::collections::hash_map::RandomState::new::KEYS::__init` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:2952:23
    = note: inside closure at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:351:25
    = note: inside `std::thread::local::lazy::LazyKeyInner::<std::cell::Cell<(u64, u64)>>::initialize::<[closure@std::collections::hash_map::RandomState::new::KEYS::__getit::{closure#0}]>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:807:25
    = note: inside `std::thread::__FastLocalKeyInner::<std::cell::Cell<(u64, u64)>>::try_initialize::<[closure@std::collections::hash_map::RandomState::new::KEYS::__getit::{closure#0}]>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:985:31
    = note: inside `std::thread::__FastLocalKeyInner::<std::cell::Cell<(u64, u64)>>::get::<[closure@std::collections::hash_map::RandomState::new::KEYS::__getit::{closure#0}]>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:968:29
    = note: inside `std::collections::hash_map::RandomState::new::KEYS::__getit` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:343:21
    = note: inside `std::thread::LocalKey::<std::cell::Cell<(u64, u64)>>::try_with::<[closure@std::collections::hash_map::RandomState::new::{closure#0}], std::collections::hash_map::RandomState>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:442:32
    = note: inside `std::thread::LocalKey::<std::cell::Cell<(u64, u64)>>::with::<[closure@std::collections::hash_map::RandomState::new::{closure#0}], std::collections::hash_map::RandomState>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:419:9
    = note: inside `std::collections::hash_map::RandomState::new` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:2955:9
    = note: inside `<std::collections::hash_map::RandomState as std::default::Default>::default` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:3025:9
    = note: inside `std::collections::HashMap::<&[u8], intaglio::Symbol>::with_capacity` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:247:53
note: inside `intaglio::bytes::SymbolTable::with_capacity` at /Users/lopopolo/dev/artichoke/intaglio/src/bytes.rs:367:36
   --> /Users/lopopolo/dev/artichoke/intaglio/src/bytes.rs:367:36
    |
367 |             map: ManuallyDrop::new(HashMap::with_capacity(capacity)),
    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `bytes::dealloc_owned_data` at tests/leak_drop/bytes.rs:5:21
   --> tests/leak_drop/bytes.rs:5:21
    |
5   |     let mut table = SymbolTable::with_capacity(0);
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside closure at tests/leak_drop/bytes.rs:4:1
   --> tests/leak_drop/bytes.rs:4:1
    |
3   |   #[test]
    |   ------- in this procedural macro expansion
4   | / fn dealloc_owned_data() {
5   | |     let mut table = SymbolTable::with_capacity(0);
6   | |     for sym in crate::vectors::byte_symbols() {
7   | |         let symbol = sym;
...   |
18  | |     }
19  | | }
    | |_^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

error: test failed, to rerun pass '--test leak_drop'

let ret = unsafe { f(s.as_mut_ptr() as *mut c_void, s.len()) };

Meta

rustc --version --verbose:

rustc 1.62.0-nightly (3f391b845 2022-04-15)
binary: rustc
commit-hash: 3f391b84552f210adec7893b50c5da74f9362ae4
commit-date: 2022-04-15
host: x86_64-apple-darwin
release: 1.62.0-nightly
LLVM version: 14.0.0
$ cargo +nightly miri --version --verbose
miri 0.1.0 (c568f32 2022-04-09)
Backtrace

<backtrace>

@lopopolo lopopolo added the C-bug Category: This is a bug. label Apr 18, 2022
@lopopolo
Copy link
Contributor Author

I gathered the flags I used in the miri invocation from:

@lopopolo
Copy link
Contributor Author

lopopolo commented Apr 18, 2022

Miri finds UB in the same place if I run with a less spicy set of -Z flags:

MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-check-number-validity" cargo +nightly miri test --test leak_drop

gives:

$ MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-check-number-validity" cargo +nightly miri test --test leak_drop
    Finished test [unoptimized + debuginfo] target(s) in 0.03s
     Running tests/leak_drop/main.rs (target/miri/x86_64-apple-darwin/debug/deps/leak_drop-59a0909d5e2ab85f)

running 5 tests
test bytes::dealloc_owned_data ... error: Undefined Behavior: 0x1aa85f is not a valid pointer
   --> /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:130:40
    |
130 |                     let ret = unsafe { f(s.as_mut_ptr() as *mut c_void, s.len()) };
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 0x1aa85f is not a valid pointer
    |
    = 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

    = note: inside closure at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:130:40
    = note: inside `std::option::Option::<unsafe extern "C" fn(*mut std::ffi::c_void, usize) -> i32>::map::<bool, [closure@std::sys::unix::rand::imp::getentropy_fill_bytes::{closure#0}]>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/option.rs:909:29
    = note: inside `std::sys::unix::rand::imp::getentropy_fill_bytes` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:125:9
    = note: inside `std::sys::unix::rand::imp::fill_bytes` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:141:12
    = note: inside `std::sys::unix::rand::hashmap_random_keys` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/rand.rs:8:9
    = note: inside `std::collections::hash_map::RandomState::new::KEYS::__init` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:2952:23
    = note: inside closure at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:351:25
    = note: inside `std::thread::local::lazy::LazyKeyInner::<std::cell::Cell<(u64, u64)>>::initialize::<[closure@std::collections::hash_map::RandomState::new::KEYS::__getit::{closure#0}]>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:807:25
    = note: inside `std::thread::__FastLocalKeyInner::<std::cell::Cell<(u64, u64)>>::try_initialize::<[closure@std::collections::hash_map::RandomState::new::KEYS::__getit::{closure#0}]>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:985:31
    = note: inside `std::thread::__FastLocalKeyInner::<std::cell::Cell<(u64, u64)>>::get::<[closure@std::collections::hash_map::RandomState::new::KEYS::__getit::{closure#0}]>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:968:29
    = note: inside `std::collections::hash_map::RandomState::new::KEYS::__getit` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:343:21
    = note: inside `std::thread::LocalKey::<std::cell::Cell<(u64, u64)>>::try_with::<[closure@std::collections::hash_map::RandomState::new::{closure#0}], std::collections::hash_map::RandomState>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:442:32
    = note: inside `std::thread::LocalKey::<std::cell::Cell<(u64, u64)>>::with::<[closure@std::collections::hash_map::RandomState::new::{closure#0}], std::collections::hash_map::RandomState>` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/thread/local.rs:419:9
    = note: inside `std::collections::hash_map::RandomState::new` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:2955:9
    = note: inside `<std::collections::hash_map::RandomState as std::default::Default>::default` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:3025:9
    = note: inside `std::collections::HashMap::<&[u8], intaglio::Symbol>::with_capacity` at /Users/lopopolo/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:247:53
note: inside `intaglio::bytes::SymbolTable::with_capacity` at /Users/lopopolo/dev/artichoke/intaglio/src/bytes.rs:367:36
   --> /Users/lopopolo/dev/artichoke/intaglio/src/bytes.rs:367:36
    |
367 |             map: ManuallyDrop::new(HashMap::with_capacity(capacity)),
    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `bytes::dealloc_owned_data` at tests/leak_drop/bytes.rs:5:21
   --> tests/leak_drop/bytes.rs:5:21
    |
5   |     let mut table = SymbolTable::with_capacity(0);
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside closure at tests/leak_drop/bytes.rs:4:1
   --> tests/leak_drop/bytes.rs:4:1
    |
3   |   #[test]
    |   ------- in this procedural macro expansion
4   | / fn dealloc_owned_data() {
5   | |     let mut table = SymbolTable::with_capacity(0);
6   | |     for sym in crate::vectors::byte_symbols() {
7   | |         let symbol = sym;
...   |
18  | |     }
19  | | }
    | |_^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

error: test failed, to rerun pass '--test leak_drop'

@lopopolo lopopolo changed the title Miri on hard mode flags UB in macos sys random Miri with strict provenance flags UB in macos impl of hashmap_random_keys Apr 18, 2022
@CAD97
Copy link
Contributor

CAD97 commented Apr 18, 2022

Context:

weak!(fn getentropy(*mut c_void, size_t) -> c_int);
getentropy
.get()
.map(|f| {
// getentropy(2) permits a maximum buffer size of 256 bytes
for s in v.chunks_mut(256) {
let ret = unsafe { f(s.as_mut_ptr() as *mut c_void, s.len()) };
if ret == -1 {
panic!("unexpected getentropy error: {}", errno());
}
}
true
})
.unwrap_or(false)
}

As best as I can tell from eyeballing the code, 0x1ac318 is not a valid pointer applies to f, not s, which comes from an input slice (and thus would've already been an issue).

I think this is just the fact that dlsym doesn't play well with strict-provenance:

// On non-ELF targets, use the dlsym approximation of weak linkage.
#[cfg(any(target_os = "macos", target_os = "ios"))]
pub(crate) use self::dlsym as weak;

pub(crate) macro dlsym {
(fn $name:ident($($t:ty),*) -> $ret:ty) => (
dlsym!(fn $name($($t),*) -> $ret, stringify!($name));
),
(fn $name:ident($($t:ty),*) -> $ret:ty, $sym:expr) => (
static DLSYM: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> =
DlsymWeak::new(concat!($sym, '\0'));
let $name = &DLSYM;
)
}

let func = mem::transmute_copy::<usize, F>(&addr);

unsafe fn fetch(name: &str) -> usize {
let name = match CStr::from_bytes_with_nul(name.as_bytes()) {
Ok(cstr) => cstr,
Err(..) => return 0,
};
libc::dlsym(libc::RTLD_DEFAULT, name.as_ptr()) as usize
}

@RalfJung
Copy link
Member

Specifically I see usize in there, which would indeed lead to a loss of provenance.

@RalfJung RalfJung added the A-miri Area: The miri tool label Apr 19, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 20, 2022
…thomcc

Replace sys/unix/weak AtomicUsize with AtomicPtr

Should fix rust-lang#96163. Can't easily test on Windows though...
@bors bors closed this as completed in 53f028d Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-miri Area: The miri tool C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants