872394 - WebAudio use-after-free [@mozilla::dom::SelfCountedReference<mozilla::dom::ScriptProcessorNode>::ForceDrop]

Status: RESOLVED WORKSFORME

(Core :: Web Audio, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

alloc & free: js/src/jsscript.cpp:1106,1112

/* Adjust the amount of memory this script source uses for source data,
   reallocating if needed. */
bool
ScriptSource::adjustDataSize(size_t nbytes)
{
    // Allocating 0 bytes has undefined behavior, so special-case it.
    if (nbytes == 0) {
        if (data.compressed != emptySource)
*           js_free(data.compressed);
        data.compressed = const_cast<unsigned char *>(emptySource);
        return true;
    }

    // |data.compressed| can be NULL.
*   void *buf = js_realloc(data.compressed, nbytes);
    if (!buf && data.compressed != emptySource)
        js_free(data.compressed);
    data.compressed = static_cast<unsigned char *>(buf);
    return !!data.compressed;
}

re-use: content/media/webaudio/AudioNode.h:88
  void ForceDrop(T* t)
  {
*   if (mRefCnt > 0) {
      mRefCnt = 0;
      t->Release();
    }
  }


Note: this happened till now only one time and am till now not able to reproduce that issue with a testcase.


Tested with m-i changeset: 131837:3a56d9a0b092

Comment 1

[Christoph Diehl [:posidron]]

Hit it now twice - only the stack for the re-use is constant.

It only happens with large testcases (each testcase has 200 calls of WebAudio functions) and a page reload after every 100th testcase.

Comment 2

[(no longer active)]

I'm changing this code slightly in bug 836599.  Would be interesting to retest after that lands.

Comment 3

[(no longer active)]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #2)
> I'm changing this code slightly in bug 836599.  Would be interesting to
> retest after that lands.

Can you please retest?

Comment 4

[Christoph Diehl [:posidron]]

I am not seeing it anymore in my latest runs.

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

\o/
Ehsan want to close fixed via bug 836599?

Comment 6

[Christoph Diehl [:posidron]]

I believe this got fixed in bug 873335. The testcase produced the same stack.

Comment 7

[Andrew McCreight [:mccr8]]

Bug 836599 was only landed in 24, so even if this is fixed by something else, 23 remains affected.  Are we going leave it enabled on 22?  Does this affect 22 otherwise?

Comment 8

[Andrew McCreight [:mccr8]]

(In reply to Christoph Diehl [:cdiehl] from comment #6)
> I believe this got fixed in bug 873335. The testcase produced the same stack.

Thanks for the update.

Comment 9

[(no longer active)]

(In reply to Andrew McCreight [:mccr8] from comment #7)
> Bug 836599 was only landed in 24, so even if this is fixed by something
> else, 23 remains affected.  Are we going leave it enabled on 22?  Does this
> affect 22 otherwise?

Web Audio is disabled on 22 and will be disabled on 23 once it gets to beta too.

And we probably need to uplift bug 836599 for 23 anyway.

Comment 10

[Christoph Diehl [:posidron]]

Hit it again.

Comment 11

[(no longer active)]

(In reply to Christoph Diehl [:cdiehl] from comment #10)
> Hit it again.

On which test case?

Comment 12

[Christoph Diehl [:posidron]]

still, no specific testcase.

Comment 13

[(no longer active)]

Does this still happen?

Comment 14

[Christoph Diehl [:posidron]]

Haven't seen it anymore lately - testing against m-i changeset: 133183:1c67a51e0fe5

Comment 15

[(no longer active)]

Cool, marking as FIXED.

Comment 16

[(no longer active)]

Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

Not tracking since we're not shipping web audio in ff23

Comment 18

[(no longer active)]

Sorry Kyle, but this bug is not actionable without a test case.  Assigning it to somebody is not going to change that situation.

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The bug is fixed ... I was just doing bookkeeping ...

Comment 20

[(no longer active)]

Well yeah.  We don't know if it's actually fixed yet, we just can't reproduce it.