Crash with cyclic channel merger
https://bugs.webkit.org/show_bug.cgi?id=139664
Summary Crash with cyclic channel merger
mark.buer+webkitbugs
Reported 2014-12-15 19:21:32 PST
This snippet of JS code run from the developer console of Safari: var audioContext; if (!audioContext) { if (window.AudioContext) { audioContext = new AudioContext(); } else if (window.webkitAudioContext) { audioContext = new webkitAudioContext(); } else { throw new Error('AudioContext missing'); } } var o = audioContext.createOscillator(); var m = audioContext.createChannelMerger(2); var d = audioContext.createDelay(); o.connect(m, 0, 1); d.connect(m, 0, 0); m.connect(d); m.connect(audioContext.destination); d.delayTime.value = 0.5; o.start(0); causes a crash with this relevant stack trace: Thread 22 Crashed:: com.apple.audio.IOThread.client 0 com.apple.WebCore 0x000000010f5b99f6 WebCore::AudioNodeOutput::updateInternalBus() + 22 1 com.apple.WebCore 0x000000010f5b991a WebCore::AudioNodeOutput::updateNumberOfChannels() + 42 2 com.apple.WebCore 0x000000010f5e59b9 WebCore::ChannelMergerNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 89 3 com.apple.WebCore 0x000000010f5b9990 WebCore::AudioNodeOutput::updateNumberOfChannels() + 160 4 com.apple.WebCore 0x000000010f5aea82 WebCore::AudioBasicProcessorNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 114 5 com.apple.WebCore 0x000000010f5b9990 WebCore::AudioNodeOutput::updateNumberOfChannels() + 160 6 com.apple.WebCore 0x000000010f5e59b9 WebCore::ChannelMergerNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 89 7 com.apple.WebCore 0x000000010f5b9990 WebCore::AudioNodeOutput::updateNumberOfChannels() + 160 8 com.apple.WebCore 0x000000010f5aea82 WebCore::AudioBasicProcessorNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 114 etc, etc 128 com.apple.WebCore 0x000000010f5aea82 WebCore::AudioBasicProcessorNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 114 129 com.apple.WebCore 0x000000010f5b9990 WebCore::AudioNodeOutput::updateNumberOfChannels() + 160 130 com.apple.WebCore 0x000000010f5e59b9 WebCore::ChannelMergerNode::checkNumberOfChannelsForInput(WebCore::AudioNodeInput*) + 89 131 com.apple.WebCore 0x000000010f5bd330 WebCore::AudioSummingJunction::updateRenderingState() + 384 132 com.apple.WebCore 0x000000010f5b5078 WebCore::AudioContext::handleDirtyAudioSummingJunctions() + 120 133 com.apple.WebCore 0x000000010f5b4fd1 WebCore::AudioContext::handlePreRenderTasks() + 129 134 com.apple.WebCore 0x000000010f5b7488 WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long) + 136 135 com.apple.WebCore 0x000000010f5b6f72 WebCore::AudioDestinationMac::render(unsigned int, AudioBufferList*) + 82 136 com.apple.WebCore 0x000000010f5b6eaf WebCore::AudioDestinationMac::inputProc(void*, unsigned int*, AudioTimeStamp const*, unsigned int, unsigned int, AudioBufferList*) + 15 137 com.apple.audio.units.Components 0x000000012182c039 AUInputElement::PullInput(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int) + 177 138 com.apple.audio.units.Components 0x000000012182b8ef AUInputFormatConverter2::InputProc(OpaqueAudioConverter*, unsigned int*, AudioBufferList*, AudioStreamPacketDescription**, void*) + 193 139 com.apple.audio.toolbox.AudioToolbox 0x00007fff8ea0c185 AudioConverterChain::CallInputProc(unsigned int) + 417 140 com.apple.audio.toolbox.AudioToolbox 0x00007fff8ea0bed1 AudioConverterChain::FillBufferFromInputProc(unsigned int*, CABufferList*) + 125 141 com.apple.audio.toolbox.AudioToolbox 0x00007fff8e9eaf19 BufferedAudioConverter::GetInputBytes(unsigned int, unsigned int&, CABufferList const*&) + 179 142 com.apple.audio.toolbox.AudioToolbox 0x00007fff8e9c9c9a CBRConverter::RenderOutput(CABufferList*, unsigned int, unsigned int&, AudioStreamPacketDescription*) + 104 143 com.apple.audio.toolbox.AudioToolbox 0x00007fff8e9eada0 BufferedAudioConverter::FillBuffer(unsigned int&, AudioBufferList&, AudioStreamPacketDescription*) + 286 144 com.apple.audio.toolbox.AudioToolbox 0x00007fff8ea0bce9 AudioConverterChain::RenderOutput(CABufferList*, unsigned int, unsigned int&, AudioStreamPacketDescription*) + 99 145 com.apple.audio.toolbox.AudioToolbox 0x00007fff8e9eada0 BufferedAudioConverter::FillBuffer(unsigned int&, AudioBufferList&, AudioStreamPacketDescription*) + 286 146 com.apple.audio.toolbox.AudioToolbox 0x00007fff8e9c94ee AudioConverterFillComplexBuffer + 292 147 com.apple.audio.units.Components 0x000000012182b7a6 AUInputFormatConverter2::PullAndConvertInput(AudioTimeStamp const&, unsigned int&, AudioBufferList&, AudioStreamPacketDescription*, bool&) + 98 148 com.apple.audio.units.Components 0x000000012182b08a AUConverterBase::RenderBus(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int) + 188 149 com.apple.audio.units.Components 0x0000000121828c61 AUBase::DoRenderBus(unsigned int&, AudioTimeStamp const&, unsigned int, AUOutputElement*, unsigned int, AudioBufferList&) + 153 150 com.apple.audio.units.Components 0x0000000121827515 AUBase::DoRender(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int, AudioBufferList&) + 423 151 com.apple.audio.units.Components 0x000000012182e878 AUHAL::AUIOProc(unsigned int, AudioTimeStamp const*, AudioBufferList const*, AudioTimeStamp const*, AudioBufferList*, AudioTimeStamp const*, void*) + 2076 152 com.apple.audio.CoreAudio 0x00007fff8bdc49db HALC_ProxyIOContext::IOWorkLoop() + 3667 153 com.apple.audio.CoreAudio 0x00007fff8bdc3add HALC_ProxyIOContext::IOThreadEntry(void*) + 97 154 com.apple.audio.CoreAudio 0x00007fff8bdc399d HALB_IOThread::Entry(void*) + 75 155 libsystem_pthread.dylib 0x00007fff8fe2b899 _pthread_body + 138 156 libsystem_pthread.dylib 0x00007fff8fe2b72a _pthread_start + 137 157 libsystem_pthread.dylib 0x00007fff8fe2ffc9 thread_start + 13 Guessing at the cause, the line at https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp#L74 probably results in a null audio bus being returned at some point (after several renders?) due to an excessive number of channels. If this guess is correct, an easy fix might be to modify https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/webaudio/AudioNodeOutput.cpp#L74 to always create an audio bus with capped number of channels. ie: m_internalBus = AudioBus::create(std::min(numberOfChannels(), AudioContext::maxNumberOfChannels()), AudioNode::ProcessingSizeInFrames);
Attachments
Alexey Proskuryakov
Comment 1 2014-12-15 23:27:41 PST
This looks like a runaway recursion to me.
mark.buer+webkitbugs
Comment 2 2014-12-16 20:21:19 PST
@Alexey That was my first thought too. But, there aren't very many stack frames in total and the final frame is different from previous frames, meaning it has reached an endpoint for the recursion. This doesn't look to be a security concern as it is a null pointer dereference (not a dangling pointer dereference). The null pointer is set when an attempt is made to create an AudioBus with too many channels as seen here https://github.com/WebKit/webkit/blob/master/Source/WebCore/platform/audio/AudioBus.cpp#L53 . Some time later, an attempt is made to dereference that null pointer, which causes the crash. A fix appears to have been applied to Blink, it's a shame it wasn't thrown over the wall to WebKit at the same time. I cannot find the changeset for this fix. A check against the channel count is performed here: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/modules/webaudio/ChannelMergerNode.cpp&q=channelmerger&sq=package:chromium&l=118 that is missing from here: https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/webaudio/ChannelMergerNode.cpp#L105 Applying this one-line change to my local checkout of WebKit confirms that it addresses the bug. I've raised a bug against the specification for this scenario of ChannelMergerNodes in a cycle: https://github.com/WebAudio/web-audio-api/issues/459 I've attached a debugging session showing that this bug is indeed a null pointer dereference (and not a dangling pointer dereference), thus substantiating my "probably not a security bug" claim from earlier: (lldb) thread step-over Process 70149 stopped * thread #17: tid = 0x1dbbbb, 0x000000010c3c3bc8 WebCore`WebCore::AudioNodeOutput::updateInternalBus(this=0x00007fd97b218f80) + 24 at AudioNodeOutput.cpp:71, name = 'com.apple.audio.IOThread.client', stop reason = step over frame #0: 0x000000010c3c3bc8 WebCore`WebCore::AudioNodeOutput::updateInternalBus(this=0x00007fd97b218f80) + 24 at AudioNodeOutput.cpp:71 68 69 void AudioNodeOutput::updateInternalBus() 70 { -> 71 if (numberOfChannels() == m_internalBus->numberOfChannels()) 72 return; 73 74 m_internalBus = AudioBus::create(numberOfChannels(), AudioNode::ProcessingSizeInFrames); (lldb) frame variable this->m_numberOfChannels (unsigned int) this->m_numberOfChannels = 34 (lldb) frame variable this->m_internalBus (WTF::RefPtr<WebCore::AudioBus>) this->m_internalBus = { m_ptr = 0x0000000000000000 } (lldb) thread step-over Process 70149 stopped * thread #17: tid = 0x1dbbbb, 0x000000010c3c3bc8 WebCore`WebCore::AudioNodeOutput::updateInternalBus(this=0x00007fd97b218f80) + 24 at AudioNodeOutput.cpp:71, name = 'com.apple.audio.IOThread.client', stop reason = EXC_BAD_ACCESS (code=1, address=0x1c) frame #0: 0x000000010c3c3bc8 WebCore`WebCore::AudioNodeOutput::updateInternalBus(this=0x00007fd97b218f80) + 24 at AudioNodeOutput.cpp:71 68 69 void AudioNodeOutput::updateInternalBus() 70 { -> 71 if (numberOfChannels() == m_internalBus->numberOfChannels()) 72 return; 73 74 m_internalBus = AudioBus::create(numberOfChannels(), AudioNode::ProcessingSizeInFrames);
Radar WebKit Bug Importer
Comment 3 2014-12-17 10:56:08 PST
Ahmad Saleem
Comment 4 2023-08-10 09:41:10 PDT
Note You need to log in before you can comment on or make changes to this bug.