Closed Bug 882956 Opened 8 years ago Closed 8 years ago

WebAudio stack-buffer-overflow crash [@void mozilla::InterleaveAndConvertBuffer<float, float>]

Categories

(Core :: Web Audio, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox23 --- unaffected
firefox24 --- fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: posidron, Assigned: shelly)

References

Details

(5 keywords, Whiteboard: [adv-main24-])

Attachments

(3 files, 1 obsolete file)

Attached file testcase
Wait 3 seconds after loading the testcase.


File: content/media/AudioSegment.cpp:24

template <class SrcT, class DestT>
static void
InterleaveAndConvertBuffer(const SrcT** aSourceChannels,
                           int32_t aLength, float aVolume,
                           int32_t aChannels,
                           DestT* aOutput)
{
  DestT* output = aOutput;
  for (int32_t i = 0; i < aLength; ++i) {
    for (int32_t channel = 0; channel < aChannels; ++channel) {
      float v = AudioSampleToFloat(aSourceChannels[channel][i])*aVolume;
*     *output = FloatToAudioSample<DestT>(v);
      ++output;
    }
  }
}


Tested with http://hg.mozilla.org/integration/mozilla-inbound/rev/ac7d9177c7ee
Attached file callstack
This code was added in bug 842243.

Shelly, can you please take a look at this?
Assignee: nobody → slin
Blocks: 842243
Sure thing.
Thanks, and let me know if you need help.  It seems like the output buffer here is smaller than what we're expecting.
When down-mixing the audio source, should pass the "output channels count" to the downmix function, not the "source channels count".

Hi Ehsan, could you review the patch? It's a small fix, I'm running the try-server in the mean while.
Attachment #762487 - Flags: review?(ehsan)
Comment on attachment 762487 [details] [diff] [review]
Fix patch for the crash in AudioSegment.cpp

Review of attachment 762487 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!  Can you please also include the test case here as a crashtest in <http://mxr.mozilla.org/mozilla-central/source/content/media/test/crashtests/> when landing?  Thanks!
Attachment #762487 - Flags: review?(ehsan) → review+
Thanks! Re-push the patch to try-server now.
Attachment #762487 - Attachment is obsolete: true
Attachment #762540 - Flags: review?(ehsan)
Attachment #762540 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/ccd298a9db28
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Whiteboard: [adv-main24-]
You need to log in before you can comment on or make changes to this bug.