Closed Bug 1407148 Opened 4 years ago Closed 4 years ago

Crash in mozilla::dom::HTMLMediaElement::InitializeDecoderForChannel

Categories

(Core :: Audio/Video: Playback, defect, P1)

58 Branch
Unspecified
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: calixte, Assigned: jwwang)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, sec-other, Whiteboard: [clouseau][adv-main58-][post-critsmash-triage])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-f6ec5b1e-f22b-4af3-bf87-e68680171009.
=============================================================

There are 15 crashes in nightly 58 starting with buildid 20171009100134. In analyzing the backtrace, the regressoin may have been introduced by patch [1] to fix bug 1402584.
The crash reason is always MOZ_RELEASE_ASSERT(mReadyState == nsIDOMHTMLMediaElement::HAVE_NOTHING).

[1] https://hg.mozilla.org/mozilla-central/rev?node=a4466933d251bca22688a859c108eb11772401c2
Flags: needinfo?(jwwang)
Priority: -- → P2
Crash Signature: [@ mozilla::dom::HTMLMediaElement::InitializeDecoderForChannel] → [@ mozilla::dom::HTMLMediaElement::InitializeDecoderForChannel] [@ mozilla::dom::HTMLMediaElement::InitializeDecoderAsClone]
This is bad which means there are still unknown call paths that fail to reset readyState to HAVE_NOTHING. In the worse case, this might be an indication of UAF.

I will add more logs/assertions to debug the crashes.
Flags: needinfo?(jwwang)
Attachment #8918157 - Flags: review?(gsquelart)
Assignee: nobody → jwwang
Comment on attachment 8918157 [details]
Bug 1407148 - add logs for debugging crashes.

https://reviewboard.mozilla.org/r/189010/#review194318

Good luck!
Attachment #8918157 - Flags: review?(gsquelart) → review+
Thanks! :)
Keywords: leave-open
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1ec29c15fdb
add logs for debugging crashes. r=gerald
Depends on: 1408482
There are 15 crashes with signature "mozilla::dom::HTMLMediaElement::AssertReadyStateIsNothing" in nightly 58 starting with buildid 20171013220204.
Crash Signature: [@ mozilla::dom::HTMLMediaElement::InitializeDecoderForChannel] [@ mozilla::dom::HTMLMediaElement::InitializeDecoderAsClone] → [@ mozilla::dom::HTMLMediaElement::InitializeDecoderForChannel] [@ mozilla::dom::HTMLMediaElement::InitializeDecoderAsClone] [@ mozilla::dom::HTMLMediaElement::AssertReadyStateIsNothing]
Depends on: 1409263
Depends on: 1409270
Depends on: 1409649
I don't see any crash reports after 20171019100107... is it possible something else fixed this?
Flags: needinfo?(jwwang)
I guess this is fixed by bug 1409649.
Flags: needinfo?(jwwang)
Keywords: leave-open
I see wildptr crashes, including EXEC wildptr crashes, going back to at least 52ESR, on the first signature if I open up to a month.

I think there are (at least) two bugs here; the one reported by calixte and an existing one.

This initially-reported (by calixte) regression may well be fixed.  If so, please close this bug, mark as 58 only affected, and clone a new bug that covers 52ESR and up (and make that one sec-critical, and this one sec-other).  As a sec-critical, that should be on the radar for possible ride-alongs once we know we have a fix.
Group: media-core-security
Flags: needinfo?(jwwang)
Priority: P2 → P1
How to open a cloned bug for the EXEC wildptr crash you mentioned?

Btw, the initially-reported issue is not a security bug, but just an assertion which checks if the readyState of the media element is in a consistent status.
(In reply to JW Wang [:jwwang] [:jw_wang] from comment #11)
> How to open a cloned bug for the EXEC wildptr crash you mentioned?

New/Clone Bug in the bottom-right of this bug..

> Btw, the initially-reported issue is not a security bug, but just an
> assertion which checks if the readyState of the media element is in a
> consistent status.

If we split off the security-sensitive items to a new bug, we could change the title on this one, and make it 'sec-other' instead of sec-critical.

(I'm looking at all the crashes in the linked-to signatures for this bug).  for example this crash: https://crash-stats.mozilla.com/report/index/9980818c-0366-4e98-84ae-fcb5f0171018 )
Blocks: 1415788
Switch to sec-other per comment 12. Also close this bug since we've fixed the assertion. We will continue to track memory issues in bug 1415788.
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(jwwang)
Keywords: sec-criticalsec-other
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Group: media-core-security → core-security-release
Note: don't open this bug until bug 1415788 is resolved
Whiteboard: [clouseau] → [clouseau][adv-main58-]
Flags: qe-verify-
Whiteboard: [clouseau][adv-main58-] → [clouseau][adv-main58-][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.