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

RESOLVED FIXED in Firefox 24

Status

()

defect
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: posidron, Assigned: shelly)

Tracking

(Blocks 1 bug, 5 keywords)

Trunk
mozilla24
x86_64
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox23 unaffected, firefox24 fixed, firefox-esr17 unaffected, b2g18 unaffected)

Details

(Whiteboard: [adv-main24-])

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

6 years ago
Posted 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
Reporter

Comment 1

6 years ago
Posted file callstack

Comment 2

6 years ago
This code was added in bug 842243.

Shelly, can you please take a look at this?
Assignee: nobody → slin
Blocks: 842243
Assignee

Comment 3

6 years ago
Sure thing.

Comment 4

6 years ago
Thanks, and let me know if you need help.  It seems like the output buffer here is smaller than what we're expecting.
Assignee

Comment 5

6 years ago
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 6

6 years ago
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+
Assignee

Comment 7

6 years ago
Thanks! Re-push the patch to try-server now.
Attachment #762487 - Attachment is obsolete: true
Attachment #762540 - Flags: review?(ehsan)

Updated

6 years ago
Attachment #762540 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/ccd298a9db28
Status: NEW → RESOLVED
Last Resolved: 6 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.