1479269 - AddressSanitizer: heap-use-after-free [@ GetBoolFlag] with READ of size 4 involving [@ SVGObserverUtils::InvalidateDirectRenderingObservers]

Status: RESOLVED INCOMPLETE

(Core :: Layout, defect, P1)

Description

[Christian Holler (:decoder)]

The attached crash information was submitted via the ASan Nightly Reporter on mozilla-central-asan-nightly revision 63.0a1-20180728220145-https://hg.mozilla.org/mozilla-central/rev/71922e0cd9e13b89db636665fc5ed2e6858c660a.

For detailed crash information, see attachment.

Comment 2

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

For some reason, FuzzManager had trouble creating the attachment, will investigate later.

Comment 3

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 6

[Andrew McCreight [:mccr8]]

The free and allocation look unrelated to the use. They are dealing with what looks like a vector backing a JS iterator, while the use is SVGObserverUtils::InvalidateDirectRenderingObservers() touching a node.

Comment 7

[Christian Holler (:decoder)]

The stacks not matching might be caused by the memory being reallocated between its free in the layout code and the use later on. In the meantime, JS might have allocated and freed the memory again. Since ASan retains memory for quite a while, that would mean quite a bit of time and memory use must have been between the free and the use in layout.

When is the InvalidateDirectRenderingObservers called? Would it be possible to call this more often to force the use-after-free to happen earlier before the memory is reallocated?

But even if the free/alloc stacks are unrelated to the use stack, maybe we can still figure this out just from that?

Comment 8

[Maire Reavy [:mreavy]]

Hey Sean, Can you find an owner for this?

Comment 9

[Sean Voisen (:svoisen)]

jwatt: Would you mind investigating this? Looks like it's touching SVGObserverUtils so might be an area you are familiar with. If not feel free to bounce the ni back to me.

Comment 10

[Mats Palmgren (inactive)]

Do we have a testcase / STR to trigger the crash?

Comment 11

[Christian Holler (:decoder)]

(In reply to Mats Palmgren (:mats) from comment #10)
> Do we have a testcase / STR to trigger the crash?

No, this is an ASan Nightly submission. If the "use" stack alone here is not enough to diagnose the problem (alloc/free stacks are from JS because the memory was likely reallocated), then we need to resolve this as incomplete.

Comment 12

[Jonathan Watt [:jwatt]]

(In reply to Christian Holler (:decoder) from comment #7)
> When is the InvalidateDirectRenderingObservers called? Would it be possible
> to call this more often to force the use-after-free to happen earlier before
> the memory is reallocated?

This isn't really anything to do with InvalidateDirectRenderingObservers. That function just happens to be the first thing that dereferences mContent under nsFrame::DestroyFrom. The problem is that the frame's nsIContent has been destroyed before it has, which violates one of our invariants. I've been unable to figure out how that might happen.

Emilio, you've been working on getting more familiar with the frame constructor code the last few months - any ideas how that could happen?

Comment 13

[Emilio Cobos Álvarez (:emilio)]

mContent is a strong pointer, so... it's not supposed to go away. I'll take a look first thing tomorrow morning.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Oh, there's no test-case, that definitely makes it harder to track down...

The fact that the memory being allocated / freed is not a DOM node or other Gecko stuff, but unrelated JIT stuff is very suspicious. Would there be any chance that the JIT is somehow corrupting the frame arena?

If the mContent pointer has gone away, then this is a refcounting bug somewhere else, presumably. But the fact that JS is not supposed to allocate in the same mozalloc arena as the rest of Gecko (IIRC) makes me thing the frame arena memory itself can be corrupt somehow... 

Does that sound unreasonable? It doesn't look trivial to somehow mprotect that to try to hunt crashes earlier though.

Comment 15

[Christian Holler (:decoder)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #14)

> The fact that the memory being allocated / freed is not a DOM node or other
> Gecko stuff, but unrelated JIT stuff is very suspicious. Would there be any
> chance that the JIT is somehow corrupting the frame arena?

The fact that JS stuff is shown under allocated/freed can simply be caused by the fact that inbetween the free of the mContent pointer and the use-after-free, the memory was reallocated to JS. ASan tries to avoid this by quarantining free'd memory for as long as possible, but if there is no memory left or the quarantine is full, then this memory will be handed out again. So it is perfectly ok for JS stuff to appear in these traces, but it is unrelated to the use-after-free.


> If the mContent pointer has gone away, then this is a refcounting bug
> somewhere else, presumably. But the fact that JS is not supposed to allocate
> in the same mozalloc arena as the rest of Gecko (IIRC) makes me thing the
> frame arena memory itself can be corrupt somehow... 

Do we use the mozalloc arenas without jemalloc? Because ASan has jemalloc disabled.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

(In reply to Christian Holler (:decoder) from comment #15)
> (In reply to Emilio Cobos Álvarez (:emilio) from comment #14)
> 
> > The fact that the memory being allocated / freed is not a DOM node or other
> > Gecko stuff, but unrelated JIT stuff is very suspicious. Would there be any
> > chance that the JIT is somehow corrupting the frame arena?
> 
> The fact that JS stuff is shown under allocated/freed can simply be caused
> by the fact that inbetween the free of the mContent pointer and the
> use-after-free, the memory was reallocated to JS. ASan tries to avoid this
> by quarantining free'd memory for as long as possible, but if there is no
> memory left or the quarantine is full, then this memory will be handed out
> again. So it is perfectly ok for JS stuff to appear in these traces, but it
> is unrelated to the use-after-free.

I see. Then this bug is probably a refcounting bug somewhere else, and I don't know how would I track it down without the different stacks :(

> > If the mContent pointer has gone away, then this is a refcounting bug
> > somewhere else, presumably. But the fact that JS is not supposed to allocate
> > in the same mozalloc arena as the rest of Gecko (IIRC) makes me thing the
> > frame arena memory itself can be corrupt somehow... 
> 
> Do we use the mozalloc arenas without jemalloc? Because ASan has jemalloc
> disabled.

Ah, didn't know that. AFAIK (and as before, talking from memory) the different arena is a mozjemalloc thing, then it makes sense.

Comment 17

[Mats Palmgren (inactive)]

I don't see this stack on crash-stats either, so no clues from there.
I don't think there's much more we can do here without STR so
I suggest we close this bug.

Comment 18

[Christian Holler (:decoder)]

(In reply to Mats Palmgren (:mats) from comment #17)
> I don't see this stack on crash-stats either, so no clues from there.

It will be very unlikely to find this signature on crash-stats as this wouldn't necessarily crash at all in a regular build.

> I suggest we close this bug.

I will mark the bug as "incomplete" for now. If I find another trace of this type with better alloc/free stacks, I will post the information here.