diff options
| -rw-r--r-- | audio/coreaudio.c | 102 | ||||
| -rw-r--r-- | hw/audio/sb16.c | 31 | ||||
| -rw-r--r-- | tests/qtest/fuzz-sb16-test.c | 17 |
3 files changed, 77 insertions, 73 deletions
diff --git a/audio/coreaudio.c b/audio/coreaudio.c index f570e1ee60..d8a21d3e50 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -26,6 +26,7 @@ #include <CoreAudio/CoreAudio.h> #include <pthread.h> /* pthread_X */ +#include "qemu/main-loop.h" #include "qemu/module.h" #include "audio.h" @@ -34,7 +35,7 @@ typedef struct coreaudioVoiceOut { HWVoiceOut hw; - pthread_mutex_t mutex; + pthread_mutex_t buf_mutex; AudioDeviceID outputDeviceID; int frameSizeSetting; uint32_t bufferCount; @@ -241,11 +242,11 @@ static void GCC_FMT_ATTR (3, 4) coreaudio_logerr2 ( #define coreaudio_playback_logerr(status, ...) \ coreaudio_logerr2(status, "playback", __VA_ARGS__) -static int coreaudio_lock (coreaudioVoiceOut *core, const char *fn_name) +static int coreaudio_buf_lock (coreaudioVoiceOut *core, const char *fn_name) { int err; - err = pthread_mutex_lock (&core->mutex); + err = pthread_mutex_lock (&core->buf_mutex); if (err) { dolog ("Could not lock voice for %s\nReason: %s\n", fn_name, strerror (err)); @@ -254,11 +255,11 @@ static int coreaudio_lock (coreaudioVoiceOut *core, const char *fn_name) return 0; } -static int coreaudio_unlock (coreaudioVoiceOut *core, const char *fn_name) +static int coreaudio_buf_unlock (coreaudioVoiceOut *core, const char *fn_name) { int err; - err = pthread_mutex_unlock (&core->mutex); + err = pthread_mutex_unlock (&core->buf_mutex); if (err) { dolog ("Could not unlock voice for %s\nReason: %s\n", fn_name, strerror (err)); @@ -273,13 +274,13 @@ static int coreaudio_unlock (coreaudioVoiceOut *core, const char *fn_name) coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw; \ ret_type ret; \ \ - if (coreaudio_lock(core, "coreaudio_" #name)) { \ + if (coreaudio_buf_lock(core, "coreaudio_" #name)) { \ return 0; \ } \ \ ret = glue(audio_generic_, name)args; \ \ - coreaudio_unlock(core, "coreaudio_" #name); \ + coreaudio_buf_unlock(core, "coreaudio_" #name); \ return ret; \ } COREAUDIO_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size), @@ -291,7 +292,10 @@ COREAUDIO_WRAPPER_FUNC(write, size_t, (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size)) #undef COREAUDIO_WRAPPER_FUNC -/* callback to feed audiooutput buffer */ +/* + * callback to feed audiooutput buffer. called without iothread lock. + * allowed to lock "buf_mutex", but disallowed to have any other locks. + */ static OSStatus audioDeviceIOProc( AudioDeviceID inDevice, const AudioTimeStamp *inNow, @@ -307,13 +311,13 @@ static OSStatus audioDeviceIOProc( coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr; size_t len; - if (coreaudio_lock (core, "audioDeviceIOProc")) { + if (coreaudio_buf_lock (core, "audioDeviceIOProc")) { inInputTime = 0; return 0; } if (inDevice != core->outputDeviceID) { - coreaudio_unlock (core, "audioDeviceIOProc(old device)"); + coreaudio_buf_unlock (core, "audioDeviceIOProc(old device)"); return 0; } @@ -323,7 +327,7 @@ static OSStatus audioDeviceIOProc( /* if there are not enough samples, set signal and return */ if (pending_frames < frameCount) { inInputTime = 0; - coreaudio_unlock (core, "audioDeviceIOProc(empty)"); + coreaudio_buf_unlock (core, "audioDeviceIOProc(empty)"); return 0; } @@ -345,7 +349,7 @@ static OSStatus audioDeviceIOProc( out += write_len; } - coreaudio_unlock (core, "audioDeviceIOProc"); + coreaudio_buf_unlock (core, "audioDeviceIOProc"); return 0; } @@ -438,7 +442,16 @@ static OSStatus init_out_device(coreaudioVoiceOut *core) return status; } - /* set Callback */ + /* + * set Callback. + * + * On macOS 11.3.1, Core Audio calls AudioDeviceIOProc after calling an + * internal function named HALB_Mutex::Lock(), which locks a mutex in + * HALB_IOThread::Entry(void*). HALB_Mutex::Lock() is also called in + * AudioObjectGetPropertyData, which is called by coreaudio driver. + * Therefore, the specified callback must be designed to avoid a deadlock + * with the callers of AudioObjectGetPropertyData. + */ core->ioprocid = NULL; status = AudioDeviceCreateIOProcID(core->outputDeviceID, audioDeviceIOProc, @@ -521,6 +534,7 @@ static void update_device_playback_state(coreaudioVoiceOut *core) } } +/* called without iothread lock. */ static OSStatus handle_voice_change( AudioObjectID in_object_id, UInt32 in_number_addresses, @@ -530,9 +544,7 @@ static OSStatus handle_voice_change( OSStatus status; coreaudioVoiceOut *core = in_client_data; - if (coreaudio_lock(core, __func__)) { - abort(); - } + qemu_mutex_lock_iothread(); if (core->outputDeviceID) { fini_out_device(core); @@ -543,7 +555,7 @@ static OSStatus handle_voice_change( update_device_playback_state(core); } - coreaudio_unlock (core, __func__); + qemu_mutex_unlock_iothread(); return status; } @@ -558,14 +570,10 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as, struct audsettings obt_as; /* create mutex */ - err = pthread_mutex_init(&core->mutex, NULL); + err = pthread_mutex_init(&core->buf_mutex, NULL); if (err) { dolog("Could not create mutex\nReason: %s\n", strerror (err)); - goto mutex_error; - } - - if (coreaudio_lock(core, __func__)) { - goto lock_error; + return -1; } obt_as = *as; @@ -584,37 +592,21 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as, if (status != kAudioHardwareNoError) { coreaudio_playback_logerr (status, "Could not listen to voice property change\n"); - goto listener_error; + return -1; } if (init_out_device(core)) { - goto device_error; + status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject, + &voice_addr, + handle_voice_change, + core); + if (status != kAudioHardwareNoError) { + coreaudio_playback_logerr(status, + "Could not remove voice property change listener\n"); + } } - coreaudio_unlock(core, __func__); return 0; - -device_error: - status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject, - &voice_addr, - handle_voice_change, - core); - if (status != kAudioHardwareNoError) { - coreaudio_playback_logerr(status, - "Could not remove voice property change listener\n"); - } - -listener_error: - coreaudio_unlock(core, __func__); - -lock_error: - err = pthread_mutex_destroy(&core->mutex); - if (err) { - dolog("Could not destroy mutex\nReason: %s\n", strerror (err)); - } - -mutex_error: - return -1; } static void coreaudio_fini_out (HWVoiceOut *hw) @@ -623,10 +615,6 @@ static void coreaudio_fini_out (HWVoiceOut *hw) int err; coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw; - if (coreaudio_lock(core, __func__)) { - abort(); - } - status = AudioObjectRemovePropertyListener(kAudioObjectSystemObject, &voice_addr, handle_voice_change, @@ -637,10 +625,8 @@ static void coreaudio_fini_out (HWVoiceOut *hw) fini_out_device(core); - coreaudio_unlock(core, __func__); - /* destroy mutex */ - err = pthread_mutex_destroy(&core->mutex); + err = pthread_mutex_destroy(&core->buf_mutex); if (err) { dolog("Could not destroy mutex\nReason: %s\n", strerror (err)); } @@ -650,14 +636,8 @@ static void coreaudio_enable_out(HWVoiceOut *hw, bool enable) { coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw; - if (coreaudio_lock(core, __func__)) { - abort(); - } - core->enabled = enable; update_device_playback_state(core); - - coreaudio_unlock(core, __func__); } static void *coreaudio_audio_init(Audiodev *dev) diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c index 5cf121fe36..60f1f75e3a 100644 --- a/hw/audio/sb16.c +++ b/hw/audio/sb16.c @@ -229,6 +229,23 @@ static void continue_dma8 (SB16State *s) control (s, 1); } +static inline int restrict_sampling_rate(int freq) +{ + if (freq < SAMPLE_RATE_MIN) { + qemu_log_mask(LOG_GUEST_ERROR, + "sampling range too low: %d, increasing to %u\n", + freq, SAMPLE_RATE_MIN); + return SAMPLE_RATE_MIN; + } else if (freq > SAMPLE_RATE_MAX) { + qemu_log_mask(LOG_GUEST_ERROR, + "sampling range too high: %d, decreasing to %u\n", + freq, SAMPLE_RATE_MAX); + return SAMPLE_RATE_MAX; + } else { + return freq; + } +} + static void dma_cmd8 (SB16State *s, int mask, int dma_len) { s->fmt = AUDIO_FORMAT_U8; @@ -244,17 +261,7 @@ static void dma_cmd8 (SB16State *s, int mask, int dma_len) int tmp = (256 - s->time_const); s->freq = (1000000 + (tmp / 2)) / tmp; } - if (s->freq < SAMPLE_RATE_MIN) { - qemu_log_mask(LOG_GUEST_ERROR, - "sampling range too low: %d, increasing to %u\n", - s->freq, SAMPLE_RATE_MIN); - s->freq = SAMPLE_RATE_MIN; - } else if (s->freq > SAMPLE_RATE_MAX) { - qemu_log_mask(LOG_GUEST_ERROR, - "sampling range too high: %d, decreasing to %u\n", - s->freq, SAMPLE_RATE_MAX); - s->freq = SAMPLE_RATE_MAX; - } + s->freq = restrict_sampling_rate(s->freq); if (dma_len != -1) { s->block_size = dma_len << s->fmt_stereo; @@ -768,7 +775,7 @@ static void complete (SB16State *s) * and FT2 sets output freq with this (go figure). Compare: * http://homepages.cae.wisc.edu/~brodskye/sb16doc/sb16doc.html#SamplingRate */ - s->freq = dsp_get_hilo (s); + s->freq = restrict_sampling_rate(dsp_get_hilo(s)); ldebug ("set freq %d\n", s->freq); break; diff --git a/tests/qtest/fuzz-sb16-test.c b/tests/qtest/fuzz-sb16-test.c index 51030cd7dc..f47a8bcdbd 100644 --- a/tests/qtest/fuzz-sb16-test.c +++ b/tests/qtest/fuzz-sb16-test.c @@ -37,6 +37,22 @@ static void test_fuzz_sb16_0x91(void) qtest_quit(s); } +/* + * This used to trigger the assert in audio_calloc + * through command 0xd4 + */ +static void test_fuzz_sb16_0xd4(void) +{ + QTestState *s = qtest_init("-M pc -display none " + "-device sb16,audiodev=none " + "-audiodev id=none,driver=none"); + qtest_outb(s, 0x22c, 0x41); + qtest_outb(s, 0x22c, 0x00); + qtest_outb(s, 0x22c, 0x14); + qtest_outb(s, 0x22c, 0xd4); + qtest_quit(s); +} + int main(int argc, char **argv) { const char *arch = qtest_get_arch(); @@ -46,6 +62,7 @@ int main(int argc, char **argv) if (strcmp(arch, "i386") == 0) { qtest_add_func("fuzz/test_fuzz_sb16/1c", test_fuzz_sb16_0x1c); qtest_add_func("fuzz/test_fuzz_sb16/91", test_fuzz_sb16_0x91); + qtest_add_func("fuzz/test_fuzz_sb16/d4", test_fuzz_sb16_0xd4); } return g_test_run(); |