...

test backend::tests::device_change::test_switch_device ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 183 filtered out

==4253==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000129db at pc 0x00010a5c5793 bp 0x7000004b91c0 sp 0x7000004b91b8
READ of size 1 at 0x6160000129db thread T6

test_destroy_duplex_stream_after_unplugging_a_nondefault_input_device

Run RUSTFLAGS="-Z sanitizer=address" cargo test test_destroy_duplex_stream_after_unplugging_a_nondefault_input_device -- --ignored --nocapture to see

...

running 1 test
Device change callback. data @ 0x603000011ff0
=================================================================
test backend::tests::device_change::test_destroy_duplex_stream_after_unplugging_a_nondefault_input_device ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 183 filtered out

==4268==ERROR: AddressSanitizer: heap-use-after-free on address 0x616000010bdb at pc 0x000103ffe793 bp 0x700007cee1c0 sp 0x700007cee1b8
READ of size 1 at 0x616000010bdb thread T5

test_destroy_duplex_stream_after_unplugging_a_default_input_device

Run RUSTFLAGS="-Z sanitizer=address" cargo test test_destroy_duplex_stream_after_unplugging_a_default_input_device -- --ignored --nocapture to see:

...

running 1 test
Device change callback. data @ 0x603000013820
=================================================================
test backend::tests::device_change::test_destroy_duplex_stream_after_unplugging_a_default_input_device ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 183 filtered out

==4292==ERROR: AddressSanitizer: heap-use-after-free on address 0x616000010bdb at pc 0x00010cc31793 bp 0x70000cd961c0 sp 0x70000cd961b8
READ of size 1 at 0x616000010bdb thread T6

test_destroy_duplex_stream_after_unplugging_a_default_output_device

Run RUSTFLAGS="-Z sanitizer=address" cargo test test_destroy_duplex_stream_after_unplugging_a_default_output_device -- --ignored --nocapture to see:

...

running 1 test
Device change callback. data @ 0x603000012110
=================================================================
test backend::tests::device_change::test_destroy_duplex_stream_after_unplugging_a_default_output_device ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 183 filtered out

==4315==ERROR: AddressSanitizer: heap-use-after-free on address 0x616000010bdb at pc 0x0001082cf793 bp 0x7000031ef1c0 sp 0x7000031ef1b8
READ of size 1 at 0x616000010bdb thread T6

C.M.Chang[:chunmin]

Assignee

Comment 2

•

5 years ago

Got a more detail log for running RUSTFLAGS="-Z sanitizer=address" cargo test test_switch_device -- --ignored --nocapture with adding a delay on the end of the test (otherwise the stacktraces won't be printed, don't know why)

running 1 test

Switch default device for Output while the stream is working.

Output devices
--------------------
 204: AppleGFXHDAEngineOutputDP:10001:0:{C315-2684-05BF0E12}
	uid: EV2750
 194: AppleHDAEngineOutput:1F,3,0,1,1:0
	uid: Headphones
  41: SoundflowerEngine:0
	uid: Soundflower (2ch)
  52: SoundflowerEngine:1
	uid: Soundflower (64ch)

Switch device for Output: 194 -> 41
Switch device for Output: 41 -> 52
Switch device for Output: 52 -> 204
Switch device for Output: 204 -> 194
=================================================================
==7236==ERROR: AddressSanitizer: heap-use-after-free on address 0x616000010bdb at pc 0x000106410723 bp 0x7000029fa1c0 sp 0x7000029fa1b8
READ of size 1 at 0x616000010bdb thread T6
    #0 0x106410722 in core::sync::atomic::atomic_load::h4dc415c6a3ce79a6 atomic.rs:2277
    #1 0x10647185c in core::sync::atomic::AtomicBool::load::hd6d654f0922c62d7 atomic.rs:404
    #2 0x1063fb16c in cubeb_coreaudio::backend::AudioUnitStream::reinit_async::_$u7b$$u7b$closure$u7d$$u7d$::hbea024c63054739e mod.rs:3270
    #3 0x1063082e2 in coreaudio_sys_utils::dispatch::create_closure_and_executor::closure_executer::haff5ead75c17e7a3 dispatch.rs:59
    #4 0x106c87441 in asan_dispatch_call_block_and_release (librustc_rt.asan.dylib:x86_64+0x41441)
    #5 0x7fff6e3f850d in _dispatch_client_callout (libdispatch.dylib:x86_64+0x350d)
    #6 0x7fff6e3fdacd in _dispatch_lane_serial_drain (libdispatch.dylib:x86_64+0x8acd)
    #7 0x7fff6e3fe451 in _dispatch_lane_invoke (libdispatch.dylib:x86_64+0x9451)
    #8 0x7fff6e407a9d in _dispatch_workloop_worker_thread (libdispatch.dylib:x86_64+0x12a9d)
    #9 0x7fff6e6526fb in _pthread_wqthread (libsystem_pthread.dylib:x86_64+0x26fb)
    #10 0x7fff6e651826 in start_wqthread (libsystem_pthread.dylib:x86_64+0x1826)

0x616000010bdb is located 91 bytes inside of 608-byte region [0x616000010b80,0x616000010de0)
freed by thread T1 here:
    #0 0x106c88ad6 in wrap_free (librustc_rt.asan.dylib:x86_64+0x42ad6)
    #1 0x10669c57a in alloc::alloc::dealloc::h2a46682b3fd1fcd4 alloc.rs:103
    #2 0x1063cbbc0 in alloc::alloc::box_free::h85e777566b08c76d alloc.rs:217
    #3 0x1062af623 in core::ptr::drop_in_place::he4a20f3e32dba16b mod.rs:174
    #4 0x10643c62b in cubeb_backend::capi::capi_stream_destroy::h504649dfea752dec capi.rs:173
    #5 0x1064e0a09 in cubeb_coreaudio::backend::tests::utils::test_ops_stream_operation::_$u7b$$u7b$closure$u7d$$u7d$::hbc15cf636f0b150a utils.rs:1091
    #6 0x1064c795a in cubeb_coreaudio::backend::tests::utils::test_ops_context_operation::hfab4167c5af0e787 utils.rs:1035
    #7 0x1064cd09f in cubeb_coreaudio::backend::tests::utils::test_ops_stream_operation::haaacc9c4ff9c81b0 utils.rs:1056
    #8 0x1061f95ca in cubeb_coreaudio::backend::tests::device_change::test_get_started_stream_in_scope::hcd049a2a814dc689 device_change.rs:165
    #9 0x1064049cc in cubeb_coreaudio::backend::tests::device_change::test_switch_device_in_scope::hb0894f5959915bd4 device_change.rs:60
    #10 0x10640dfa8 in cubeb_coreaudio::backend::tests::device_change::test_switch_device::hf2026957a4208330 device_change.rs:31
    #11 0x1061ffa70 in cubeb_coreaudio::backend::tests::device_change::test_switch_device::_$u7b$$u7b$closure$u7d$$u7d$::h7e363590f88f7d2c device_change.rs:29
    #12 0x10627e643 in core::ops::function::FnOnce::call_once::h0da49dbfb7e04f06 function.rs:232
    #13 0x10676d5ed in _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h09f25e49c6b2dde5 boxed.rs:1016
    #14 0x1068627ea in __rust_maybe_catch_panic lib.rs:86
    #15 0x106787363 in test::run_test::run_test_inner::_$u7b$$u7b$closure$u7d$$u7d$::he7e26757f8f77360 lib.rs:539
    #16 0x10676058a in std::sys_common::backtrace::__rust_begin_short_backtrace::h1ec69ba5fcb050d2 backtrace.rs:129
    #17 0x1067652da in std::panicking::try::do_call::h3f0071cc5ab7da00 panicking.rs:303
    #18 0x1068627ea in __rust_maybe_catch_panic lib.rs:86
    #19 0x106765df5 in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h059274226e6540b3 mod.rs:474
    #20 0x10684fd7d in _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h9d68ab0c0b6f49ed boxed.rs:1016
    #21 0x10686212d in std::sys::unix::thread::Thread::new::thread_start::h18be3f46590c0640 boxed.rs:1016
    #22 0x7fff6e655e64 in _pthread_start (libsystem_pthread.dylib:x86_64+0x5e64)
    #23 0x7fff6e65183a in thread_start (libsystem_pthread.dylib:x86_64+0x183a)

previously allocated by thread T1 here:
    #0 0x106c8898d in wrap_malloc (librustc_rt.asan.dylib:x86_64+0x4298d)
    #1 0x10669c3c4 in alloc::alloc::alloc::hf6ad4692150492f5 alloc.rs:81
    #2 0x10669c16b in alloc::alloc::exchange_malloc::h873d61ec2e163377 alloc.rs:203
    #3 0x10654c166 in _$LT$cubeb_coreaudio..backend..AudioUnitContext$u20$as$u20$cubeb_backend..traits..ContextOps$GT$::stream_init::h174f9c68d49e1f3b mod.rs:2159
    #4 0x10643bbef in cubeb_backend::capi::capi_stream_init::h354e09e23cca1513 capi.rs:155
    #5 0x1064e0163 in cubeb_coreaudio::backend::tests::utils::test_ops_stream_operation::_$u7b$$u7b$closure$u7d$$u7d$::hbc15cf636f0b150a utils.rs:1072
    #6 0x1064c795a in cubeb_coreaudio::backend::tests::utils::test_ops_context_operation::hfab4167c5af0e787 utils.rs:1035
    #7 0x1064cd09f in cubeb_coreaudio::backend::tests::utils::test_ops_stream_operation::haaacc9c4ff9c81b0 utils.rs:1056
    #8 0x1061f95ca in cubeb_coreaudio::backend::tests::device_change::test_get_started_stream_in_scope::hcd049a2a814dc689 device_change.rs:165
    #9 0x1064049cc in cubeb_coreaudio::backend::tests::device_change::test_switch_device_in_scope::hb0894f5959915bd4 device_change.rs:60
    #10 0x10640dfa8 in cubeb_coreaudio::backend::tests::device_change::test_switch_device::hf2026957a4208330 device_change.rs:31
    #11 0x1061ffa70 in cubeb_coreaudio::backend::tests::device_change::test_switch_device::_$u7b$$u7b$closure$u7d$$u7d$::h7e363590f88f7d2c device_change.rs:29
    #12 0x10627e643 in core::ops::function::FnOnce::call_once::h0da49dbfb7e04f06 function.rs:232
    #13 0x10676d5ed in _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h09f25e49c6b2dde5 boxed.rs:1016
    #14 0x1068627ea in __rust_maybe_catch_panic lib.rs:86
    #15 0x106787363 in test::run_test::run_test_inner::_$u7b$$u7b$closure$u7d$$u7d$::he7e26757f8f77360 lib.rs:539
    #16 0x10676058a in std::sys_common::backtrace::__rust_begin_short_backtrace::h1ec69ba5fcb050d2 backtrace.rs:129
    #17 0x1067652da in std::panicking::try::do_call::h3f0071cc5ab7da00 panicking.rs:303
    #18 0x1068627ea in __rust_maybe_catch_panic lib.rs:86
    #19 0x106765df5 in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h059274226e6540b3 mod.rs:474
    #20 0x10684fd7d in _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h9d68ab0c0b6f49ed boxed.rs:1016
    #21 0x10686212d in std::sys::unix::thread::Thread::new::thread_start::h18be3f46590c0640 boxed.rs:1016
    #22 0x7fff6e655e64 in _pthread_start (libsystem_pthread.dylib:x86_64+0x5e64)
    #23 0x7fff6e65183a in thread_start (libsystem_pthread.dylib:x86_64+0x183a)

Thread T6 created by T4 here:
    <empty stack>

Thread T4 created by T1 here:
    #0 0x106c82b8a in wrap_pthread_create (librustc_rt.asan.dylib:x86_64+0x3cb8a)
    #1 0x7fff6e402742 in _dispatch_root_queue_poke_slow (libdispatch.dylib:x86_64+0xd742)
    #2 0x7fff366deaf5 in HALB_DispatchQueue::InstallMIGServer(unsigned int, unsigned int, unsigned int (*)(mach_msg_header_t*, mach_msg_header_t*)) (CoreAudio:x86_64+0x3f9af5)
    #3 0x7fff3660e26a in HALC_ProxySystem::HALC_ProxySystem() (CoreAudio:x86_64+0x32926a)
    #4 0x7fff3661e0af in HALC_ProxyObjectMap::_CreateSystemObject() (CoreAudio:x86_64+0x3390af)
    #5 0x7fff3661d48a in HALC_ProxyObjectMap::CopyObjectByObjectID(unsigned int) (CoreAudio:x86_64+0x33848a)
    #6 0x7fff362e7812 in HALSystem::InitializeDevices() (CoreAudio:x86_64+0x2812)
    #7 0x7fff362e686c in HALSystem::CheckOutInstance() (CoreAudio:x86_64+0x186c)
    #8 0x7fff362ec390 in AudioObjectGetPropertyDataSize (CoreAudio:x86_64+0x7390)
    #9 0x106619685 in cubeb_coreaudio::backend::tests::utils::test_get_all_devices::hfc2293ff551a6aa8 utils.rs:255
    #10 0x10661a9b1 in cubeb_coreaudio::backend::tests::utils::test_get_devices_in_scope::hc73873ced910c1db utils.rs:291
    #11 0x1064046b0 in cubeb_coreaudio::backend::tests::device_change::test_switch_device_in_scope::hb0894f5959915bd4 device_change.rs:41
    #12 0x10640dfa8 in cubeb_coreaudio::backend::tests::device_change::test_switch_device::hf2026957a4208330 device_change.rs:31
    #13 0x1061ffa70 in cubeb_coreaudio::backend::tests::device_change::test_switch_device::_$u7b$$u7b$closure$u7d$$u7d$::h7e363590f88f7d2c device_change.rs:29
    #14 0x10627e643 in core::ops::function::FnOnce::call_once::h0da49dbfb7e04f06 function.rs:232
    #15 0x10676d5ed in _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h09f25e49c6b2dde5 boxed.rs:1016
    #16 0x1068627ea in __rust_maybe_catch_panic lib.rs:86
    #17 0x106787363 in test::run_test::run_test_inner::_$u7b$$u7b$closure$u7d$$u7d$::he7e26757f8f77360 lib.rs:539
    #18 0x10676058a in std::sys_common::backtrace::__rust_begin_short_backtrace::h1ec69ba5fcb050d2 backtrace.rs:129
    #19 0x1067652da in std::panicking::try::do_call::h3f0071cc5ab7da00 panicking.rs:303
    #20 0x1068627ea in __rust_maybe_catch_panic lib.rs:86
    #21 0x106765df5 in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::h059274226e6540b3 mod.rs:474
    #22 0x10684fd7d in _$LT$alloc..boxed..Box$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h9d68ab0c0b6f49ed boxed.rs:1016
    #23 0x10686212d in std::sys::unix::thread::Thread::new::thread_start::h18be3f46590c0640 boxed.rs:1016
    #24 0x7fff6e655e64 in _pthread_start (libsystem_pthread.dylib:x86_64+0x5e64)
    #25 0x7fff6e65183a in thread_start (libsystem_pthread.dylib:x86_64+0x183a)

Thread T1 created by T0 here:
    #0 0x106c82b8a in wrap_pthread_create (librustc_rt.asan.dylib:x86_64+0x3cb8a)
    #1 0x106861dd7 in std::sys::unix::thread::Thread::new::h586ae1a3fbb68ccd thread.rs:68
    #2 0x106786cba in test::run_test::run_test_inner::h41a17006bde78306 mod.rs:492
    #3 0x1067852dd in test::run_test::hc2b0fa225f1cdfde lib.rs:503
    #4 0x10678127c in test::run_tests::hf9fe773408722b8d lib.rs:301
    #5 0x1067733a5 in test::console::run_tests_console::h257d38a897830738 console.rs:280
    #6 0x10677e9a2 in test::test_main::h0ece35b24fdccd5b lib.rs:122
    #7 0x10677f9ae in test::test_main_static::h03d43955d3e6f66b lib.rs:141
    #8 0x106733dd7 in cubeb_coreaudio::main::ha80bf2604bb83103 (cubeb_coreaudio-8efcc4bcc1a06d9a:x86_64+0x100564dd7)
    #9 0x1066c5a1d in std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::h46ca11651ad8e411 rt.rs:67
    #10 0x10685bc07 in std::panicking::try::do_call::hd7d5ebb621afe41a panicking.rs:303
    #11 0x1068627ea in __rust_maybe_catch_panic lib.rs:86
    #12 0x10685c4fb in std::rt::lang_start_internal::hf660b1f8d8c56d9b panicking.rs:281
    #13 0x1066c5985 in std::rt::lang_start::hdcbaaa8cfc30d74d rt.rs:67
    #14 0x106733e01 in main (cubeb_coreaudio-8efcc4bcc1a06d9a:x86_64+0x100564e01)
    #15 0x7fff6e4517fc in start (libdyld.dylib:x86_64+0x1a7fc)

SUMMARY: AddressSanitizer: heap-use-after-free atomic.rs:2277 in core::sync::atomic::atomic_load::h4dc415c6a3ce79a6
Shadow bytes around the buggy address:
  0x1c2c00002120: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2c00002130: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2c00002140: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2c00002150: fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c2c00002160: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x1c2c00002170: fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd
  0x1c2c00002180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2c00002190: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2c000021a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c2c000021b0: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x1c2c000021c0: 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
==7236==ABORTING
error: test failed, to rerun pass '--lib'

Caused by:
  process didn't exit successfully: `/Users/cchang/Work/cubeb-coreaudio-rs/target/debug/deps/cubeb_coreaudio-8efcc4bcc1a06d9a test_switch_device --ignored --nocapture` (signal: 6, SIGABRT: process abort signal)

C.M.Chang[:chunmin]

Assignee

Comment 3

•

5 years ago

Got a more detail log for running RUSTFLAGS="-Z sanitizer=address" cargo test test_switch_device -- --ignored --nocapture with adding a delay on the end of the test (otherwise the stacktraces won't be printed, don't know why)

running 1 test

Switch default device for Output while the stream is working.

Output devices
--------------------
 204: AppleGFXHDAEngineOutputDP:10001:0:{C315-2684-05BF0E12}
	uid: EV2750
 194: AppleHDAEngineOutput:1F,3,0,1,1:0
	uid: Headphones
  41: SoundflowerEngine:0
	uid: Soundflower (2ch)
  52: SoundflowerEngine:1
	uid: Soundflower (64ch)

Switch device for Output: 194 -> 41
Switch device for Output: 41 -> 52
Switch device for Output: 52 -> 204
Switch device for Output: 204 -> 194
=================================================================
==7236==ERROR: AddressSanitizer: heap-use-after-free on address 0x616000010bdb at pc 0x000106410723 bp 0x7000029fa1c0 sp 0x7000029fa1b8
READ of size 1 at 0x616000010bdb thread T6
    #0 0x106410722 in core::sync::atomic::atomic_load::h4dc415c6a3ce79a6 atomic.rs:2277
    #1 0x10647185c in core::sync::atomic::AtomicBool::load::hd6d654f0922c62d7 atomic.rs:404
    #2 0x1063fb16c in cubeb_coreaudio::backend::AudioUnitStream::reinit_async::_$u7b$$u7b$closure$u7d$$u7d$::hbea024c63054739e mod.rs:3270
    #3 0x1063082e2 in coreaudio_sys_utils::dispatch::create_closure_and_executor::closure_executer::haff5ead75c17e7a3 dispatch.rs:59
    #4 0x106c87441 in asan_dispatch_call_block_and_release (librustc_rt.asan.dylib:x86_64+0x41441)
    #5 0x7fff6e3f850d in _dispatch_client_callout (libdispatch.dylib:x86_64+0x350d)
    #6 0x7fff6e3fdacd in _dispatch_lane_serial_drain (libdispatch.dylib:x86_64+0x8acd)
    #7 0x7fff6e3fe451 in _dispatch_lane_invoke (libdispatch.dylib:x86_64+0x9451)
    #8 0x7fff6e407a9d in _dispatch_workloop_worker_thread (libdispatch.dylib:x86_64+0x12a9d)
    #9 0x7fff6e6526fb in _pthread_wqthread (libsystem_pthread.dylib:x86_64+0x26fb)
    #10 0x7fff6e651826 in start_wqthread (libsystem_pthread.dylib:x86_64+0x1826)

I dig a bit deeper today. The 0x616000010bdb here is destroy_pending: AtomicBool in AudioUnitStream at mod.rs:3270
https://github.com/ChunMinChang/cubeb-coreaudio-rs/blob/7fe03b4201160f84aad1c12536affe6b05f8839f/src/backend/mod.rs#L3270

I suspect there is a stream-reinit-task posted to the global task queue in the cubeb context (aka AudioUnitContext::serial_queue) while stream-destroy-task is being executed in the global task queue, or cubeb_stream_destroy and audiounit_property_listener_callback get called at the same time and stream-reinit-task is posted after stream-destroy-task is posted.

Suppose there are 3 different threads

M: the Main thread running the test_switch_device test
T: the thread running the task in the global task queue (AudioUnitContext::serial_queue)
D: the device-changed callback thread (whereaudiounit_property_listener_callback is on)

The timeline should be like this

M: Start running test_switch_device
M: Initialize cubeb context
M: Initialize cubeb stream
M: Start cubeb stream
M: Change the default output device
The default device is changed: The following works might be run at the same time
- M: Get notification of the device changed
  - Do 5 again until it's repeated X times
- D: If the stream-reinit-task is pending/being run, skip this event. Otherwise, post an async stream-reinit-task to the global task queue
  - T: Start executing the stream-reinit-task
The following works might be run at the same time:
- After the default output device is changed X times, the test is about to be finished, so
  M start destroying cubeb stream, which posts a sync stream-destroy-task to the the global task queue. Then M is blocked until the stream-destroy-task is done.
- While M is executing, the D is fired for the last device-changed event and then posts an async stream-reinit-task
If stream-destroy-task is posted before stream-reinit-task, T will execute stream-destroy-task first
T: Finishing the stream-destroy-task
The following works might be run at the same time:
- M: Destroy cubeb context
- T: Pick and run the stream-reinit-task after the cubeb stream is destroyed
  - T: Touching destroy_pending leads to a crash

At step 10, touching the destroy_pending in a destroyed cubeb stream causes the crash.

C.M.Chang[:chunmin]

Assignee

Comment 4

•

5 years ago

•

Edited

Got some suggestions from Alex:

In addition to what you have, you could use the if stm_guard.destroy_pending.load(Ordering::SeqCst) inside the main body of reinit_async() in a way that we don't async_dispatch another reinit closure if this flag is true. In this case, you would also need synchronization between destroy() and reinit_async().

If the solution is the above I guess the c-api is not protected by this problem, probably we haven't tried the same test.

C.M.Chang[:chunmin]

Assignee

Comment 5

•

5 years ago

•

Edited

Attached patch UAF-demo.diff — Details — Splinter Review

I can make this happen manually with this patch in version 7fe03b4, by running cargo test test_stream_tester -- --ignored --nocapture, with switching device manually and dropping the cubeb context, stream by the test command, at the proper times.

This patch adds delays to force the context-switch at a proper time to demonstrate what happens and use dbg! to touch the memory that is free after cubeb context and stream is destroyed.

The device can be switched by clicking the sound device icon on the main UI bar then select one device on the system. To drop the cubeb context and stream, it needs to manually enter the correct command in the test_stream_tester test.

The whole log for the demo (cargo test test_stream_tester -- --ignored --nocapture) is:

running 1 test
commands:
	'q': quit
	'c': create a stream
	'd': destroy a stream
	's': start the created stream
	't': stop the created stream
c
Select stream type:
1) Input 2) Output 3) In-Out Duplex 4) Back
2 // Create a output stream
Stream 0x7fed3a9122b0 created.
commands:
	'q': quit
	'c': create a stream
	'd': destroy a stream
	's': start the created stream
	't': stop the created stream
d // destroy the created output stream
1) <AudioUnitStream> destroy: Force context-switch. Please fire a device-change event now!
// Please manually change the default output device here!
2) <AudioUnitStream> reinit_async: Sneak here when destroy_pending is false. Then force context-switch
// Force to back to destroy-task, from reinit-task
3) <AudioUnitStream> destroy: Executing destroy-task.
4) <AudioUnitStream> destroy: destroy-task is done.
5) <AudioUnitStream> reinit_async: Before executing reinit-task,
	force context-switch to terminate the stream and context.
	(back to test-main thread, press 'q')
Stream 0x7f9bd2e18500 destroyed.
commands:
	'q': quit
	'c': create a stream
	'd': destroy a stream
	's': start the created stream
	't': stop the created stream
q // cubeb context will be dropped
Quit.
No need to destroy stream.
6) <AudioUnitContext> drop: context is about to be dropped.
Wait 10 seconds before ending this manual test.
7) <AudioUnitStream> reinit_async: Executing reinit-task. (after stream is destroyed?)
[src/backend/mod.rs:3289] &stm_guard = AudioUnitStream {
    context: AudioUnitContext {
        _ops: error: test failed, to rerun pass '--lib'

Caused by:
  process didn't exit successfully: `/Users/cchang/Work/cubeb-coreaudio-rs/target/debug/deps/cubeb_coreaudio-dc2fede9d64566bd test_stream_tester --ignored --nocapture` (signal: 11, SIGSEGV: invalid memory reference)

C.M.Chang[:chunmin]

Assignee

Updated

•

5 years ago

URL: https://github.com/ChunMinChang/cubeb...

C.M.Chang[:chunmin]

Assignee

Comment 6

•

5 years ago

•

Edited

Attached patch UAF-demo-c.diff — Details — Splinter Review

This is reproducible in C backend as well. By applying this patch that adds some delays at the proper time, the UAF can be reproduced via cubeb-test. The delays are added in the same place exactly as those in the Attachment 9126802 [details] [diff].

% ./cubeb-test   
Log level is DISABLED
Init cubeb backend: audiounit
collection device type is UNKNOWN
stream_init succeed
state is CUBEB_STATE_STARTED
stream_start succeed
press `q` to abort or `h` for help
q
state is CUBEB_STATE_STOPPED
1) <audiounit_stream_destroy> Force context-switch. Please fire a device-change event now!
// Need to manually change the default output device
2) <audiounit_reinit_stream_async> Sneak here when destroy_pending is 0. Then force context-switch.
3) <audiounit_stream_destroy> Executing destroy-task.
4) <audiounit_stream_destroy> destroy-task is done.
5) <audiounit_reinit_stream_async> Before executing reinit-task,
	force context-switch to terminate the stream and context.
6) <audiounit_destroy> context is dropped.
Wait 10 seconds before ending this manual test.
7) <audiounit_reinit_stream_async> Executing reinit-task. (after stream is destroyed?)
8) touch stm->output_unit: 0x0 // This is actually UAF!

C.M.Chang[:chunmin]

Assignee

Comment 7

•

5 years ago

(In reply to C.M.Chang[:chunmin] from comment #4)

I guess the c-api is not protected by this problem, probably we haven't tried the same test.

Yes, this is reproducible in both C and Rust backend. The steps-to-reproduce are in comment 5 and comment 6.

In addition to what you have, you could use the if stm_guard.destroy_pending.load(Ordering::SeqCst) inside the main body of reinit_async() in a way that we don't async_dispatch another reinit closure if this flag is true. In this case, you would also need synchronization between destroy() and reinit_async().

Actually, I've tried this at first. But it doesn't seem to work (Not sure if I understand the suggestion correctly though). Since the reinit-task and destroy-task are racing, destroy_pending might be false even if destroy is called.

Another way I could think is to move the ownership of the task queue to the cubeb stream, instead of the cubeb context. The task will be created in the stream, instead of the cubeb context. Then when the stream is being destroyed, we can cancel all the pending tasks. However, there is Dispatch Queue API that can do that. We may need to create a task-queue class by our own (and it needs to make sure it won't regress the data race fixed in bug 1598413).

C.M.Chang[:chunmin]

Assignee

Comment 8

•

5 years ago

Attached file Bug 1614971 - Revise messages in test_switch_device. r?padenot — Details

Move the test log to the first line of the switch-device test. It will
be clearer about what's going on when the error log is printed.

C.M.Chang[:chunmin]

Assignee

Comment 9

•

5 years ago

Attached file Bug 1614971 - Fix the UAF. r?padenot — Details

The UAF is actually caused by a data race between the stream-reinit task
on the device-changed callback thread, and the stream-destroy task on
the main audio thread. When the device is changed upon the stream is
about to be destroyed, the stream-reinit task can be posted to the
global task queue just right after the stream-destroy task in that task
queue. As a result, the stream-reinit task will be executed after the
stream is destroyed and cause a used-after-free error.

The solution here is to unregister the device-changed callback before
the stream is destroyed, on the main audio thread. The calls for
unregistering device-changed callbacks will block the current thread
until all the pending device-changed callbacks finish, and stop firing
any callbacks in the future. This makes sure the following things

No device-changed callback thread will run while the stream is ready
to be destroyed.
No stream-reinit task can be appended to the task queue

As a result,

There is no device-changed callback thread that races with the task
thread running the stream-destroy task
There is no stream-reinit task appended after the stream-destroy task.

(Note that the running data callbacks will be stopped in the
stream-destroy task, by stopping the running AudioUnits, before
destroying the stream.)

Depends on D63368

C.M.Chang[:chunmin]

Assignee

Comment 10

•

5 years ago

•

Edited

Comment on attachment 9127642 [details]
Bug 1614971 - Fix the UAF. r?padenot

Security Approval Request

How easily could an exploit be constructed based on the patch?: It's not hard to infer when the data race causing an UAF can happen by reading this patch carefully.
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
Which older supported branches are affected by this flaw?: For new Rust backend, it's FF74. For C backend, it exists for a long time.
If not all supported branches, which bug introduced the flaw?: None
Do you have backports for the affected branches?: Yes
If not, how different, hard to create, and risky will they be?: For Rust cubeb backend, this patch can be uplifted to the FF74 easily.
For C cubeb backend, it's easy to import this change from the Rust backend to C backend.

However, the cubeb code are in public github repos.

main cubeb repo: https://github.com/kinetiknz/cubeb
Rust cubeb on Mac OSx: https://github.com/ChunMinChang/cubeb-coreaudio-rs

The update will be submitted in github repos instead of being updated in the gecko directly. I am not sure how to hide the security vulnerability properly when updating the code. (One thing I can do is to update the code without commit messages or comments. The comments can be added in other separated commits anyway.)

How likely is this patch to cause regressions; how much testing does it need?: It's a small change without doing anything that can change the behavior. The change here is to destroy the audio stream properly to prevent system callback from being executed after the stream is destroyed.

Attachment #9127642 - Flags: sec-approval?

C.M.Chang[:chunmin]

Assignee

Updated

•

5 years ago

Attachment #9127641 - Flags: sec-approval?

Daniel Veditz [:dveditz]

Updated

•

5 years ago

Keywords: sec-high

Julien Cristau [:jcristau]

Updated

•

5 years ago

status-firefox73: --- → wontfix

status-firefox74: --- → affected

status-firefox75: --- → affected

status-firefox-esr68: --- → affected

tracking-firefox74: --- → +

tracking-firefox75: --- → +

tracking-firefox-esr68: --- → 74+

Tom Ritter [:tjr]

Comment 11

•

5 years ago

Comment on attachment 9127642 [details]
Bug 1614971 - Fix the UAF. r?padenot

sec-approved to land AFTER

Change the commit message's title to something other than 'Fix the UAF'. perhaps 'Ensure object is fully deleted' or something
Remove the rest of the commit message (it's preserved in this bug's comments)
Remove the comments from the commit
Do not land the test change until mid-way through next cycle

I don't see a backport for esr68 but you said you had one, so please ensure esr68 gets it.

Attachment #9127642 - Flags: sec-approval?

Attachment #9127642 - Flags: sec-approval+

Attachment #9127642 - Flags: approval-mozilla-esr68+

Attachment #9127642 - Flags: approval-mozilla-beta+

Tom Ritter [:tjr]

Comment 12

•

5 years ago

Comment on attachment 9127641 [details]
Bug 1614971 - Revise messages in test_switch_device. r?padenot

I'm minus-ing the test change because it should not land until ~3/25

Attachment #9127641 - Flags: sec-approval? → sec-approval-

C.M.Chang[:chunmin]

Assignee

Comment 13

•

5 years ago

Attached file Bug 1614971 - Update cubeb-coreaudio to 4acd802. r?padenot — Details

Pick commits:

4acd802: Destroy the stream properly
54d950a: Run the tests in the subcrate (#51)
132d209: Clean up clippy warnings and errors (#49)

C.M.Chang[:chunmin]

Assignee

Comment 14

•

5 years ago

(In reply to Tom Ritter [:tjr] (ni for response to sec-[approval|rating|advisories|cve]) from comment #11)

Comment on attachment 9127642 [details]
Bug 1614971 - Fix the UAF. r?padenot

sec-approved to land AFTER

Change the commit message's title to something other than 'Fix the UAF'. perhaps 'Ensure object is fully deleted' or something

Remove the rest of the commit message (it's preserved in this bug's comments)

Remove the comments from the commit

The code will be imported in D63725, instead of D63369. D63369 is created to hide the review in the public git repo, since the code is in a third-party library developed by Mozilla.

The commit message is simplied to Destroy the stream properly without any explanation. The title of D63725, Update cubeb-coreaudio to 4acd802, is a regular update title, as what we used to have in the past.

Can I carry the sec-approval from D63369 to D63725? Or I need to ask a sec-approval again?

Do not land the test change until mid-way through next cycle

Sure.

I don't see a backport for esr68 but you said you had one, so please ensure esr68 gets it.

The fix is under review in another github repo: https://github.com/kinetiknz/cubeb/pull/574. It will be imported to gecko once it's merge in the repo's master branch. It should be similar to what we have in D63725. I'll ask a sec-approval again then.

Flags: needinfo?(tom)

Tom Ritter [:tjr]

Comment 15

•

5 years ago

No, you don't need to ask for sec-approval again for either.

Flags: needinfo?(tom)

Ryan VanderMeulen [:RyanVM]

Comment 16

•

5 years ago

Is this ready to land? 74 goes to RC next week.

Flags: needinfo?(cchang)

C.M.Chang[:chunmin]

Assignee

Comment 17

•

5 years ago

Comment on attachment 9128247 [details]
Bug 1614971 - Update cubeb-coreaudio to 4acd802. r?padenot

Beta/Release Uplift Approval Request

User impact if declined: The users could get an used-after-free error.
Is this code covered by automated tests?: No
Has the fix been verified in Nightly?: No
Needs manual test from QE?: No
If yes, steps to reproduce:
List of other uplifts needed: None
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): The risk is low. It's a small change without doing anything that can change the behavior. The change here is to destroy the audio stream properly to prevent system callback from being executed after the stream is destroyed.
String changes made/needed:

Flags: needinfo?(cchang)

Attachment #9128247 - Flags: approval-mozilla-beta?

C.M.Chang[:chunmin]

Assignee

Comment 18

•

5 years ago

(In reply to Ryan VanderMeulen [:RyanVM] from comment #16)

Is this ready to land? 74 goes to RC next week.

Yes, I am going to land it.

C.M.Chang[:chunmin]

Assignee

Comment 19

•

5 years ago

(In reply to Tom Ritter [:tjr] (ni for response to sec-[approval|rating|advisories|cve]) from comment #11)

Comment on attachment 9127642 [details]
Bug 1614971 - Fix the UAF. r?padenot
...
I don't see a backport for esr68 but you said you had one, so please ensure esr68 gets it.

Paul, can you help me to merge https://github.com/kinetiknz/cubeb/pull/574 ? I don't have right to do so if the fix should be uplifted to esr 68.

Flags: needinfo?(padenot)

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 20

•

5 years ago

https://hg.mozilla.org/integration/autoland/rev/5106e7c7b90b18b84865cbb64993c17d7a7fb84a
https://hg.mozilla.org/mozilla-central/rev/5106e7c7b90b

Group: media-core-security → core-security-release

Status: ASSIGNED → RESOLVED

Closed: 5 years ago

status-firefox75: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla75

Matthew Gregan [:kinetik]

Comment 21

•

5 years ago

(In reply to C.M.Chang[:chunmin] from comment #19)

Paul, can you help me to merge https://github.com/kinetiknz/cubeb/pull/574 ? I don't have right to do so if the fix should be uplifted to esr 68.

I've merged it now. Paul had already reviewed it, so it was just waiting on the merge.

It looks like I sent you an invite for cubeb repo access a while ago, but it must've gone missing. I'll PM you a new invite link.

Flags: needinfo?(padenot)

Julien Cristau [:jcristau]

Comment 22

•

5 years ago

Comment on attachment 9127642 [details]
Bug 1614971 - Fix the UAF. r?padenot

moving approval to the right patch.

Attachment #9127642 - Flags: approval-mozilla-beta+

Julien Cristau [:jcristau]

Updated

•

5 years ago

Attachment #9128247 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 23

•

5 years ago

uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/40b874cbfe8b

What shall land for ESR68? The cubeb-coreaudio update will fail to apply, the patch with approval for ESR68 mentions the UAF in the commit message

status-firefox74: affected → fixed

Flags: needinfo?(cchang)

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 24

•

5 years ago

uplift

Comment 21 and a discussion with jcristau guided to what needed to be uplifted: https://hg.mozilla.org/releases/mozilla-esr68/rev/5e8b71ce4e88fcdda726a0695c7b582c6a7fc481

status-firefox-esr68: affected → fixed

Flags: needinfo?(cchang)

Bogdan Maris, Desktop Test Engineering

Updated

•

5 years ago

Flags: qe-verify-

Whiteboard: [post-critsmash-triage]

Sylvestre Ledru [:Sylvestre]

Updated

•

5 years ago

Blocks: asan-maintenance

C.M.Chang[:chunmin]

Assignee

Comment 25

•

5 years ago

I am not sure if this bug blocks bug 863846.

This errors are found by running Asan in the test of a third-party audio library used and maintained by Mozilla. The error happens when users unplug or switch the current output audio device. I don't think those tests are run in Firefox's test, and this won't be detected in the build process. Is there a better category, instead of asan build, to track this bug ?

Flags: needinfo?(sledru)

Sylvestre Ledru [:Sylvestre]

Comment 26

•

5 years ago

ok, makes sense!

No longer blocks: asan-maintenance

Flags: needinfo?(sledru)

Tom Ritter [:tjr]

Updated

•

5 years ago

Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main74+][adv-esr68.6+]

Tom Ritter [:tjr]

Comment 27

•

5 years ago

Attached file advisory.txt (obsolete) — Details

Tom Ritter [:tjr]

Comment 28

•

5 years ago

Attached file advisory.txt (obsolete) — Details

Attachment #9131375 - Attachment is obsolete: true

Tom Ritter [:tjr]

Comment 29

•

5 years ago

Attached file advisory.txt — Details

Attachment #9131380 - Attachment is obsolete: true

Tom Ritter [:tjr]

Updated

•

5 years ago

Alias: CVE-2020-6807

Daniel Veditz [:dveditz]

Updated

•

5 years ago

Group: core-security-release

UAF-demo.diff 5 years ago C.M.Chang[:chunmin] 3.93 KB, patch		Details \| Diff \| Splinter Review
UAF-demo-c.diff 5 years ago C.M.Chang[:chunmin] 2.66 KB, patch		Details \| Diff \| Splinter Review
Bug 1614971 - Revise messages in test_switch_device. r?padenot 5 years ago C.M.Chang[:chunmin] 47 bytes, text/x-phabricator-request	tjr : sec-approval-	Details \| Review
Bug 1614971 - Fix the UAF. r?padenot 5 years ago C.M.Chang[:chunmin] 47 bytes, text/x-phabricator-request	tjr : approval-mozilla-esr68+ tjr : sec-approval+	Details \| Review
Bug 1614971 - Update cubeb-coreaudio to 4acd802. r?padenot 5 years ago C.M.Chang[:chunmin] 47 bytes, text/x-phabricator-request	jcristau : approval-mozilla-beta+	Details \| Review
advisory.txt 5 years ago Tom Ritter [:tjr] 261 bytes, text/plain		Details
advisory.txt 5 years ago Tom Ritter [:tjr] 271 bytes, text/plain		Details
advisory.txt 5 years ago Tom Ritter [:tjr] 284 bytes, text/plain		Details

Fix heap-use-after-free errors found by AddressSanitizer in cubeb-coreaudio

test_switch_device

test_destroy_duplex_stream_after_unplugging_a_nondefault_input_device

test_destroy_duplex_stream_after_unplugging_a_default_input_device

test_destroy_duplex_stream_after_unplugging_a_default_output_device

Security Approval Request

Beta/Release Uplift Approval Request