OSX: use after free in `thread_local` when a TLS location isn't initialized until destruction of another TLS location

[mtak-]

Moved from Amanieu/parking_lot#114 because the issue is not specific to parking_lot.

Repro:

• toolchain: rustc 1.33.0-nightly (c2d381d 2019-01-10)
• OSX 10.14
• RUSTFLAGS="-Z sanitizer=address" cargo run

use std::thread;

struct A;

impl Drop for A {
    fn drop(&mut self) {
        THREAD_STRING.with(|s| {
            println!("{:?}", s);
        })
    }
}

thread_local! {
    static THREAD_STRING: String = String::from("hello there");
}

thread_local! {
    static THREAD_A: A = A;
}

fn main() {
    thread::spawn(|| {
        THREAD_A.with(|_| {});
    })
    .join()
    .unwrap();
}

If thread is changed to access THREAD_STRING before exiting, then the ASAN error goes away.

Address Sanitizer Log

==39852==ERROR: AddressSanitizer: heap-use-after-free on address 0x610000008089 at pc 0x00010c736a3f bp 0x70000fd46790 sp 0x70000fd45f40
READ of size 1 at 0x610000008089 thread T1
    #0 0x10c736a3e in __asan_memcpy (lib__rustc__clang_rt.asan_osx_dynamic.dylib:x86_64+0x56a3e)
    #1 0x10c66c6a6 in core::ptr::read::h3b49a10a9618a90a ptr.rs:575
    #2 0x10c66c0b2 in core::ptr::swap_nonoverlapping_one::he9532edd02cf97c7 ptr.rs:359
    #3 0x10c670d4c in core::mem::swap::hdf87706b2f446181 mem.rs:653
    #4 0x10c6716ed in core::mem::replace::h9ad7bc2c5ad8c60d mem.rs:711
    #5 0x10c665af2 in _$LT$core..cell..Cell$LT$T$GT$$GT$::replace::h5e8fd793dd71bad7 cell.rs:432
    #6 0x10c6658f4 in _$LT$core..cell..Cell$LT$T$GT$$GT$::set::hbc93a146608ff6a2 cell.rs:389
    #7 0x10c6596a6 in std::thread::local::fast::destroy_value::h7198d56987ed7071 local.rs:397
    #8 0x7fff7cc45d7e in tlv_finalize (libdyld.dylib:x86_64+0x16d7e)
    #9 0x7fff7ce386ae in _pthread_tsd_cleanup (libsystem_pthread.dylib:x86_64+0x36ae)
    #10 0x7fff7ce3b6b0 in _pthread_exit (libsystem_pthread.dylib:x86_64+0x66b0)
    #11 0x7fff7ce38347 in _pthread_body (libsystem_pthread.dylib:x86_64+0x3347)
    #12 0x7fff7ce3b2a6 in _pthread_start (libsystem_pthread.dylib:x86_64+0x62a6)
    #13 0x7fff7ce37424 in thread_start (libsystem_pthread.dylib:x86_64+0x2424)

0x610000008089 is located 73 bytes inside of 192-byte region [0x610000008040,0x610000008100)
freed by thread T1 here:
    #0 0x10c73967d in wrap_free (lib__rustc__clang_rt.asan_osx_dynamic.dylib:x86_64+0x5967d)
    #1 0x7fff7ce386ae in _pthread_tsd_cleanup (libsystem_pthread.dylib:x86_64+0x36ae)
    #2 0x7fff7ce3b6b0 in _pthread_exit (libsystem_pthread.dylib:x86_64+0x66b0)
    #3 0x7fff7ce38347 in _pthread_body (libsystem_pthread.dylib:x86_64+0x3347)
    #4 0x7fff7ce3b2a6 in _pthread_start (libsystem_pthread.dylib:x86_64+0x62a6)
    #5 0x7fff7ce37424 in thread_start (libsystem_pthread.dylib:x86_64+0x2424)

previously allocated by thread T1 here:
    #0 0x10c7394c3 in wrap_malloc (lib__rustc__clang_rt.asan_osx_dynamic.dylib:x86_64+0x594c3)
    #1 0x7fff7cc32611 in tlv_allocate_and_initialize_for_key (libdyld.dylib:x86_64+0x3611)
    #2 0x7fff7cc32440 in tlv_get_addr (libdyld.dylib:x86_64+0x3440)
    #3 0x10c67aa3d in std::sys_common::thread_info::set::h515643561df38c4d local.rs:296
    #4 0x10c64c6e9 in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::he15d93b7c71cc10d mod.rs:466
    #5 0x10c64d3c0 in _$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h77b2d26d5e0b55f2 boxed.rs:734
    #6 0x10c67df6b in std::sys::unix::thread::Thread::new::thread_start::h0b558311a1802cc0 thread.rs:14
    #7 0x7fff7ce3833c in _pthread_body (libsystem_pthread.dylib:x86_64+0x333c)
    #8 0x7fff7ce3b2a6 in _pthread_start (libsystem_pthread.dylib:x86_64+0x62a6)
    #9 0x7fff7ce37424 in thread_start (libsystem_pthread.dylib:x86_64+0x2424)

Thread T1 created by T0 here:
    #0 0x10c73143d in wrap_pthread_create (lib__rustc__clang_rt.asan_osx_dynamic.dylib:x86_64+0x5143d)
    #1 0x10c67dc3c in std::sys::unix::thread::Thread::new::hcbffd486872edde6 thread.rs:69
    #2 0x10c64b494 in std::thread::Builder::spawn_unchecked::h3923920731000341 mod.rs:488
    #3 0x10c64cd0f in std::thread::Builder::spawn::h7e37ea2a6d1dc69b mod.rs:382
    #4 0x10c64a58c in std::thread::spawn::h77287ca628b2c83b mod.rs:610
    #5 0x10c65458c in parking_lot_test::main::h36e62a83a5d578be main.rs:22
    #6 0x10c64db4d in std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::h38b6bb7d7da5478f rt.rs:64
    #7 0x10c67be17 in std::panicking::try::do_call::hbeca4642433c0ede panicking.rs:297
    #8 0x10c67e55e in __rust_maybe_catch_panic lib.rs:92
    #9 0x10c67c7ec in std::rt::lang_start_internal::h8e4e8fac3cfbbe81 rt.rs:48
    #10 0x10c64dabe in std::rt::lang_start::h9a9dc7a97c4ea753 rt.rs:64
    #11 0x10c6546c1 in main (parking_lot_test:x86_64+0x10000e6c1)
    #12 0x7fff7cc46084 in start (libdyld.dylib:x86_64+0x17084)

SUMMARY: AddressSanitizer: heap-use-after-free (lib__rustc__clang_rt.asan_osx_dynamic.dylib:x86_64+0x56a3e) in __asan_memcpy
Shadow bytes around the buggy address:
  0x1c2000000fc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2000000fd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2000000fe0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2000000ff0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2000001000: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
=>0x1c2000001010: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2000001020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2000001030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2000001040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2000001050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2000001060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==39852==ABORTING
Illegal instruction: 4

[jonas-schievink]

Might be related to / caused by #28129

[mtak-]

Looking at the source code for tlv_finalize and _tlv_atexit, it sure looks like UB to call _tlv_atexit during tlv_finalize which lldb shows is happening in this example.

This is worse than just not running tls destructors for the main thread. It is unsound AFAICT.

[alexcrichton]

Thanks for debugging this @mtak-! I wonder if we can fix this on our end? We might be able to prevent new TLS slots from being initialized after a thread has exited perhaps?

[mtak-]

I don't know how to reliably detect that a thread is exiting using the APIs that OSX provides (there could be a way). The logic in Apple's threadLocalVariables.c looks similar to what already exists in register_dtor_fallback, except that register_dtor_fallback handles destructor registering during thread exit properly AFAICT. Perhaps we could call into that here.

FWIW C++ has similar problems with "static local" thread locals when compiled by appleclang.

#include <iostream>

struct A {
    int x;
    ~A();
};

thread_local A thread_a{3557};

A::~A() {
    static thread_local std::string thread_string{"hello there long enough string"};

    // next line triggers a call to _tlv_atexit during tlv_finalize, and makes ASAN unhappy
    std::cout << thread_string << std::endl;
}

int main() {
    std::cout << thread_a.x << std::endl;
    return 0;
}

[alexcrichton]

Makes sense yeah, perhaps as a fallback solution we could just fix this issue for Rust-spawned threads? For those we know exactly when they're exiting (or at least when user-written Rust code has ceased executing) and that may be enough too! Skipping _tlv_atexit may also be reasonable and we already do that on a number of other platforms too