Skip to content

Commit f3eb5c8

Browse files
authored
fix(coreaudio): fix undefined behavior causing sanitizer crashes (#1052)
* fix(coreaudio): make data_size mutable in audio_devices() to fix sanitizer crashes In audio_devices(), data_size was declared as immutable but AudioObjectGetPropertyDataSize needs to write the actual property size into it. This caused undefined behavior that sanitizers (ASAN/TSAN) detected as writes to immutable memory. This bug was introduced in commit ed9d643 when migrating from coreaudio-rs to objc2-core-audio. The old raw pointer code hid the UB with an unsafe cast (&data_size as *const _ as *mut _), but the new NonNull API properly enforces mutability contracts. The fix changes: - let data_size = 0u32; → let mut data_size = 0u32; - NonNull::from(&data_size) → NonNull::from(&mut data_size) Fixes crashes during device enumeration under AddressSanitizer and ThreadSanitizer on macOS. * fix(coreaudio): make all data_size variables mutable in device.rs Fixed 7 more instances of the same UB pattern where data_size was declared immutable but passed to CoreAudio APIs that write to it: In set_sample_rate() (lines 64, 81, 128): - AudioObjectGetPropertyData writes the actual data size back - Both calls to get sample rates and sample rate ranges In Device::id() (line 376): - AudioObjectGetPropertyData for kAudioDevicePropertyDeviceUID In Device::supported_configs() (lines 420, 471): - AudioObjectGetPropertyDataSize writes the required buffer size - AudioObjectGetPropertyData confirms the size In Device::default_config() (line 598): - AudioObjectGetPropertyData for stream format All of these were causing sanitizer crashes because the CoreAudio APIs expect mutable references (the size parameter is 'inout'). * ci: add sanitizer builds to catch undefined behavior Added comprehensive sanitizer testing using matrix strategy: - AddressSanitizer (ASAN) for memory safety bugs - ThreadSanitizer (TSAN) for race conditions and threading issues - Runs on both macOS (CoreAudio) and Linux (ALSA) These sanitizers run the existing test suite with runtime instrumentation to catch undefined behavior that normal builds miss. This would have caught the data_size mutability bugs fixed in the previous commits.
1 parent 76b18c5 commit f3eb5c8

File tree

3 files changed

+112
-20
lines changed

3 files changed

+112
-20
lines changed

.github/workflows/sanitizers.yml

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
name: Sanitizers
2+
3+
on:
4+
push:
5+
branches: [master]
6+
paths-ignore:
7+
- "**.md"
8+
- "docs/**"
9+
- "LICENSE*"
10+
- ".gitignore"
11+
pull_request:
12+
branches: [master]
13+
paths-ignore:
14+
- "**.md"
15+
- "docs/**"
16+
- "LICENSE*"
17+
- ".gitignore"
18+
workflow_dispatch:
19+
20+
jobs:
21+
sanitizers:
22+
strategy:
23+
fail-fast: false
24+
matrix:
25+
include:
26+
- os: macOS-latest
27+
target: aarch64-apple-darwin
28+
sanitizer: address
29+
name: ASAN-macOS
30+
31+
- os: macOS-latest
32+
target: aarch64-apple-darwin
33+
sanitizer: thread
34+
name: TSAN-macOS
35+
36+
- os: ubuntu-latest
37+
target: x86_64-unknown-linux-gnu
38+
sanitizer: address
39+
name: ASAN-Linux
40+
41+
- os: ubuntu-latest
42+
target: x86_64-unknown-linux-gnu
43+
sanitizer: thread
44+
name: TSAN-Linux
45+
46+
name: ${{ matrix.name }}
47+
runs-on: ${{ matrix.os }}
48+
steps:
49+
- uses: actions/checkout@v5
50+
51+
- name: Cache Linux audio packages
52+
if: runner.os == 'Linux'
53+
uses: awalsh128/cache-apt-pkgs-action@latest
54+
with:
55+
packages: libasound2-dev libjack-jackd2-dev libjack-jackd2-0 libdbus-1-dev
56+
57+
- name: Install macOS dependencies
58+
if: runner.os == 'macOS'
59+
run: brew install llvm
60+
61+
- name: Install nightly Rust with rust-src
62+
uses: dtolnay/rust-toolchain@nightly
63+
with:
64+
components: rust-src
65+
66+
- name: Rust Cache
67+
uses: Swatinem/rust-cache@v2
68+
with:
69+
key: ${{ matrix.name }}
70+
71+
- name: Run tests with sanitizer
72+
env:
73+
RUSTFLAGS: -Zsanitizer=${{ matrix.sanitizer }}
74+
RUSTDOCFLAGS: -Zsanitizer=${{ matrix.sanitizer }}
75+
TSAN_OPTIONS: ${{ matrix.sanitizer == 'thread' && 'second_deadlock_stack=1' || '' }}
76+
run: |
77+
cargo +nightly test \
78+
-Zbuild-std \
79+
--target ${{ matrix.target }} \
80+
--lib \
81+
--verbose
82+
83+
- name: Upload sanitizer logs
84+
if: failure()
85+
uses: actions/upload-artifact@v4
86+
with:
87+
name: ${{ matrix.name }}-logs
88+
path: |
89+
*.log.*
90+
/tmp/asan.*
91+
/tmp/tsan.*
92+
if-no-files-found: ignore

src/host/coreaudio/macos/device.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,14 @@ fn set_sample_rate(
6262
mElement: kAudioObjectPropertyElementMaster,
6363
};
6464
let mut sample_rate: f64 = 0.0;
65-
let data_size = mem::size_of::<f64>() as u32;
65+
let mut data_size = mem::size_of::<f64>() as u32;
6666
let status = unsafe {
6767
AudioObjectGetPropertyData(
6868
audio_device_id,
6969
NonNull::from(&property_address),
7070
0,
7171
null(),
72-
NonNull::from(&data_size),
72+
NonNull::from(&mut data_size),
7373
NonNull::from(&mut sample_rate).cast(),
7474
)
7575
};
@@ -79,14 +79,14 @@ fn set_sample_rate(
7979
if sample_rate as u32 != target_sample_rate.0 {
8080
// Get available sample rate ranges.
8181
property_address.mSelector = kAudioDevicePropertyAvailableNominalSampleRates;
82-
let data_size = 0u32;
82+
let mut data_size = 0u32;
8383
let status = unsafe {
8484
AudioObjectGetPropertyDataSize(
8585
audio_device_id,
8686
NonNull::from(&property_address),
8787
0,
8888
null(),
89-
NonNull::from(&data_size),
89+
NonNull::from(&mut data_size),
9090
)
9191
};
9292
coreaudio::Error::from_os_status(status)?;
@@ -99,7 +99,7 @@ fn set_sample_rate(
9999
NonNull::from(&property_address),
100100
0,
101101
null(),
102-
NonNull::from(&data_size),
102+
NonNull::from(&mut data_size),
103103
NonNull::new(ranges.as_mut_ptr()).unwrap().cast(),
104104
)
105105
};
@@ -126,15 +126,15 @@ fn set_sample_rate(
126126
// Send sample rate updates back on a channel.
127127
let sample_rate_handler = move || {
128128
let mut rate: f64 = 0.0;
129-
let data_size = mem::size_of::<f64>() as u32;
129+
let mut data_size = mem::size_of::<f64>() as u32;
130130

131131
let result = unsafe {
132132
AudioObjectGetPropertyData(
133133
audio_device_id,
134134
NonNull::from(&sample_rate_address),
135135
0,
136136
null(),
137-
NonNull::from(&data_size),
137+
NonNull::from(&mut data_size),
138138
NonNull::from(&mut rate).cast(),
139139
)
140140
};
@@ -427,7 +427,7 @@ impl Device {
427427

428428
// CFString is copied from the audio object, use wrap_under_create_rule
429429
let mut uid: *mut CFString = std::ptr::null_mut();
430-
let data_size = size_of::<*mut CFString>() as u32;
430+
let mut data_size = size_of::<*mut CFString>() as u32;
431431

432432
// SAFETY: AudioObjectGetPropertyData is documented to write a CFString pointer
433433
// for kAudioDevicePropertyDeviceUID. We check the status code before use.
@@ -437,7 +437,7 @@ impl Device {
437437
NonNull::from(&property_address),
438438
0,
439439
null(),
440-
NonNull::from(&data_size),
440+
NonNull::from(&mut data_size),
441441
NonNull::from(&mut uid).cast(),
442442
)
443443
};
@@ -471,13 +471,13 @@ impl Device {
471471

472472
unsafe {
473473
// Retrieve the devices audio buffer list.
474-
let data_size = 0u32;
474+
let mut data_size = 0u32;
475475
let status = AudioObjectGetPropertyDataSize(
476476
self.audio_device_id,
477477
NonNull::from(&property_address),
478478
0,
479479
null(),
480-
NonNull::from(&data_size),
480+
NonNull::from(&mut data_size),
481481
);
482482
check_os_status(status)?;
483483

@@ -488,7 +488,7 @@ impl Device {
488488
NonNull::from(&property_address),
489489
0,
490490
null(),
491-
NonNull::from(&data_size),
491+
NonNull::from(&mut data_size),
492492
NonNull::new(audio_buffer_list.as_mut_ptr()).unwrap().cast(),
493493
);
494494
check_os_status(status)?;
@@ -522,13 +522,13 @@ impl Device {
522522
// See https://github.com/thestk/rtaudio/blob/master/RtAudio.cpp#L1369C1-L1375C39
523523

524524
property_address.mSelector = kAudioDevicePropertyAvailableNominalSampleRates;
525-
let data_size = 0u32;
525+
let mut data_size = 0u32;
526526
let status = AudioObjectGetPropertyDataSize(
527527
self.audio_device_id,
528528
NonNull::from(&property_address),
529529
0,
530530
null(),
531-
NonNull::from(&data_size),
531+
NonNull::from(&mut data_size),
532532
);
533533
check_os_status(status)?;
534534

@@ -540,7 +540,7 @@ impl Device {
540540
NonNull::from(&property_address),
541541
0,
542542
null(),
543-
NonNull::from(&data_size),
543+
NonNull::from(&mut data_size),
544544
NonNull::new(ranges.as_mut_ptr()).unwrap().cast(),
545545
);
546546
check_os_status(status)?;
@@ -649,13 +649,13 @@ impl Device {
649649

650650
unsafe {
651651
let mut asbd: AudioStreamBasicDescription = mem::zeroed();
652-
let data_size = mem::size_of::<AudioStreamBasicDescription>() as u32;
652+
let mut data_size = mem::size_of::<AudioStreamBasicDescription>() as u32;
653653
let status = AudioObjectGetPropertyData(
654654
self.audio_device_id,
655655
NonNull::from(&property_address),
656656
0,
657657
null(),
658-
NonNull::from(&data_size),
658+
NonNull::from(&mut data_size),
659659
NonNull::from(&mut asbd).cast(),
660660
);
661661
default_config_error_from_os_status(status)?;

src/host/coreaudio/macos/enumerate.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ unsafe fn audio_devices() -> Result<Vec<AudioDeviceID>, OSStatus> {
2626
};
2727
}
2828

29-
let data_size = 0u32;
29+
let mut data_size = 0u32;
3030
let status = AudioObjectGetPropertyDataSize(
3131
kAudioObjectSystemObject as AudioObjectID,
3232
NonNull::from(&property_address),
3333
0,
3434
null(),
35-
NonNull::from(&data_size),
35+
NonNull::from(&mut data_size),
3636
);
3737
try_status_or_return!(status);
3838

@@ -45,7 +45,7 @@ unsafe fn audio_devices() -> Result<Vec<AudioDeviceID>, OSStatus> {
4545
NonNull::from(&property_address),
4646
0,
4747
null(),
48-
NonNull::from(&data_size),
48+
NonNull::from(&mut data_size),
4949
NonNull::new(audio_devices.as_mut_ptr()).unwrap().cast(),
5050
);
5151
try_status_or_return!(status);

0 commit comments

Comments
 (0)