1410982 - Assertion failure: mPromisesForOperation.IsEmpty(), at /builds/worker/workspace/build/src/dom/media/GraphDriver.cpp:555

Status: NEW

(Core :: Web Audio, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: trigger.html

Testcase found while fuzzing mozilla-central rev ce1a86d3b4db.  Testcase requires the fuzzpriv extension which can be found here:
https://github.com/MozillaSecurity/domfuzz/tree/master/dom/extension

Further, the attached testcase could only be reproduced on an EC2 spot instance with no sound card.

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: oscillator-ended-1.html

Comment 2

[Jason Kratzer [:jkratzer]]

Attachment: log_minidump.txt

Comment 3

[Jason Kratzer [:jkratzer]]

Attachment: log_stderr.txt

Comment 4

[Nils Ohlmeier [:drno]]

Marking P2 because of crash.

Alex could you have an initial look at what is going on here?

Comment 5

[Alex Chronopoulos [:achronop]]

Jason, can you try a patch for me?

Comment 6

[Jason Kratzer [:jkratzer]]

(In reply to Alex Chronopoulos [:achronop] from comment #5)
> Jason, can you try a patch for me?

Certainly.  Feel free to pass it along and I'll test.

Comment 7

[Alex Chronopoulos [:achronop]]

Attachment: Bug1410982-protect-mPromisesForOperation.patch

Can you please check if you still repro the crash with this patch on?

Comment 8

[Jason Kratzer [:jkratzer]]

(In reply to Alex Chronopoulos [:achronop] from comment #7)
> Created attachment 8925873 [details] [diff] [review]
> Bug1410982-protect-mPromisesForOperation.patch
> 
> Can you please check if you still repro the crash with this patch on?

Alex, I cannot reproduce the issue with the patch applied.

Comment 9

[Paul Adenot (:padenot)]

Comment on attachment 8925873 [details] [diff] [review]
Bug1410982-protect-mPromisesForOperation.patch

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

Yes. Please write a proper commit message before landing.

Comment 10

[Jean-Yves Avenard [:jya]]

Comment on attachment 8925873 [details] [diff] [review]
Bug1410982-protect-mPromisesForOperation.patch

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

::: dom/media/GraphDriver.cpp
@@ +553,5 @@
>  AudioCallbackDriver::~AudioCallbackDriver()
>  {
> +#ifdef DEBUG
> +  {
> +    MonitorAutoLock mon(mGraphImpl->GetMonitor());

To me this points to a serious problem in the AudioCallbackDriver code.

mPromisesForOperation is an array owned by the AudioCallbackDriver and not shared with anything else.

That you could have a race in the destructor to access this member, one that requires the monitor to be held, indicates that another thread is still using the AudioCallbackDriver even after the destructor is being called.

That likely points to a UAF.

Comment 11

[Jean-Yves Avenard [:jya]]

Comment on attachment 8925873 [details] [diff] [review]
Bug1410982-protect-mPromisesForOperation.patch

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

sorry, I have to remove this review, because as it is it makes no sense.

If the fix requires the code to hold the monitor to read a private member in the destructor then it means you have a UAF on that object (it is accessed in another thread even after the destructor has been called).
If you don't have a UAF, then requiring the monitor shouldn't be required.

In any case, there's a problem that this change doesn't fix, just hiding the effect.

Comment 12

[Intermittent Failures Robot]

1 failures in 2561 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1

Platform breakdown:

• linux32: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1410982&startday=2019-03-18&endday=2019-03-24&tree=all

Comment 13

[Intermittent Failures Robot]

1 failures in 4331 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-central: 1

Platform breakdown:

• linux64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1410982&startday=2019-11-25&endday=2019-12-01&tree=all

Comment 14

[Noemi Erli[:noemi_erli]]

This started to spike on autoland after https://bugzilla.mozilla.org/show_bug.cgi?id=1598117 was landed:
Range: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&searchStr=mda&fromchange=6fbe1157d09a8c9916f05fcacc57db036dead23c&tochange=e33d46f958b127135ef628f7b93e62b95e97f69b

Karl, could you please take a look here?

Comment 15

[Karl Tomlinson (:karlt)]

I've requested a backout of the change for bug 1598117, thanks. The failures in the spike look a little different from this bug.

Comment 16

[Intermittent Failures Robot]

2 failures in 5144 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 2

Platform breakdown:

• windows7-32: 1
• linux64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1410982&startday=2019-12-02&endday=2019-12-08&tree=all

Comment 17

[Intermittent Failures Robot]

1 failures in 4582 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-central: 1

Platform breakdown:

• linux64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1410982&startday=2019-12-16&endday=2019-12-22&tree=all

Comment 18

[Intermittent Failures Robot]

1 failures in 4045 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1

Platform breakdown:

• macosx1014-64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1410982&startday=2020-02-17&endday=2020-02-23&tree=all