770218 - Shutdown crash in DOMSVGTransformList::IsAnimValList with deterministicgc() and watchpoints

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase (causes a shutdown crash)

1. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi
2. Run Firefox
3. Load the testcase.
4. Quit Firefox.

Result: shutdown crash [@ mozilla::DOMSVGTransformList::IsAnimValList]

Likely tickled by the current leakiness of deterministicgc(), which is bug 769015. But this seems more serious than a leak.

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Jesse Ruderman]

Might be debug-only.

Comment 3

[Andrew McCreight [:mccr8]]

The immediate cause is a double-unlink problem: when you unlink DOMSVGTransformList twice, you end up doing a null-deref.  That's easy enough to fix.

But it does seem odd that the GC and the CC aren't agreeing on whether the transform list is alive or not...

Comment 4

[Andrew McCreight [:mccr8]]

This probably isn't too big a deal.  The GC thinking something is alive that the CC thinks is dead causes double-unlinking at worst.  The SVGTransformList is being held alive via a watchpoint, so I suspect some watchpoint-related shenanigans.

Comment 5

[Andrew McCreight [:mccr8]]

Talking to Bill, I think the problem is just the GC not running during shutdown in this mode.  That would explain the GC weirdness, making this not really a security bug.  I'm going to wait for him to confirm that before opening this up.  I'll just use this bug to fix the double unlink issue.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: null check in case of double unlink

Comment 7

[Andrew McCreight [:mccr8]]

Try run looked good.

https://tbpl.mozilla.org/?tree=Try&rev=9a8ad9fdc466

Comment 8

[Jonathan Watt [:jwatt]]

Comment on attachment 643070 [details] [diff] [review]
null check in case of double unlink

DOMSVGNumberList.cpp and DOMSVGTransformList.cpp should also get the same treatment. r=jwatt with that.

Comment 9

[Andrew McCreight [:mccr8]]

Thanks for pointing out the additional classes!  I fixed all 3 in that directory.  The other unlink functions looked okay to me.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f86536e718e8

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f86536e718e8

Comment 11

[Andrew McCreight [:mccr8]]

I don't see any crashes like the stack trace here, so it probably isn't worth backporting.