Closed Bug 803086 Opened 7 years ago Closed 7 years ago
Crash in mozilla::Media
Pipeline Transmit::Process Video Chunk
This bug was filed from the Socorro interface and is report bp-2b8c9fea-f335-40d6-a553-837d42121018 . ============================================================= Turning on Video streaming crashes in today's (18th Oct) nightly build. STR: 1) Visit http://people.mozilla.com/~anarayanan/webrtc/pc_test.html with the relevant prefs set to true. 2) Click Start 3) Share the Camera => Nightly Crashes. Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dac5700acf8b&tochange=cb573b9307e5 Top of stack: 0 XUL mozilla::MediaPipelineTransmit::ProcessVideoChunk ImageContainer.h:72 1 XUL mozilla::MediaPipelineTransmit::PipelineListener::NotifyQueuedTrackChanges MediaPipeline.cpp:538 2 XUL mozilla::MediaStreamGraphImpl::ExtractPendingInput MediaStreamGraph.cpp:612 3 XUL mozilla::MediaStreamGraphImpl::RunThread MediaStreamGraph.cpp:1392
Even though the testcase is public we should mark a security issue as such.
Known upstream issue with SRTP, already reported by Christoph. There's a patch for upstream; I should apply the patch here; it didn't get included when we landed SRTP in m-c. The read-past-end of this doesn't generate any actual leak of the data and was already noted in the source; so I don't feel this is a security bug.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 790083
This still happens in today's nightly (2012-10-19): https://crash-stats.mozilla.com/report/index/bp-034ec2e6-8cbb-4e38-a0fe-6d6a12121019 Built with: http://hg.mozilla.org/mozilla-central/rev/0ff60bfb3442 which includes the changeset landed on bug 790083.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Comment on attachment 672781 [details] callstack This callstack is another bug. The original report is still extant. See also https://crash-stats.mozilla.com/report/index/bp-5e4ab5ed-7a7f-4586-ac2b-4c65e2121019
Attachment #672781 - Attachment is obsolete: true
This is really wallpaper over the bug until we figure out what and why it's happening, or if NULL images are actually allowed here
Attachment #673221 - Attachment is obsolete: true
Comment on attachment 673243 [details] [diff] [review] Protect against NULL image chunks in NotifyQueuedTrackChanges() When I modified getUserMedia to fix video insertion, I took roc's advice to send NULL images until the first one is available (the alternative was to insert a black frame, because otherwise MediaStreams will block until video is up, and audio will queue in the meantime.) However, apparently there was always an unstated requirement that NotifyQueuedTrackChanges() can give you null images. This should be documented. Not easily testable without modifying NotifyPull() for video to force NULL frames. Only an issue since I landed the patch for bug 801843 which added NULL frames. We'll want to uplift this with bug 801843
Attachment #673243 - Flags: review?(ekr)
Comment on attachment 673243 [details] [diff] [review] Protect against NULL image chunks in NotifyQueuedTrackChanges() Review of attachment 673243 [details] [diff] [review]: ----------------------------------------------------------------- lgtm
Attachment #673243 - Flags: review?(ekr) → review+
(In reply to Randell Jesup [:jesup] from comment #6) > This is really wallpaper over the bug until we figure out what and why it's > happening, or if NULL images are actually allowed here Randell, shouldn't we keep this bug open then?
That patch was obsoleted. Fixed is correct.
Verified on 10/23 build by trying out the test case described in comment 0 a few times. Couldn't reproduce the crash.
You need to log in before you can comment on or make changes to this bug.