Closed Bug 770218 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: SVG, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: jruderman, Assigned: mccr8)

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(3 files)

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.
Attached file stack trace
Might be debug-only.
Crash Signature: [@ mozilla::DOMSVGTransformList::IsAnimValList] → [@ mozilla::DOMSVGTransformList::IsAnimValList] [@ mozilla::DOMSVGTransformList::cycleCollection::UnlinkImpl]
Assignee: nobody → continuation
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...
Group: core-security
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.
Summary: Shutdown crash in DOMSVGTransformList::IsAnimValList with deterministicgc() → Shutdown crash in DOMSVGTransformList::IsAnimValList with deterministicgc() and watchpoints
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.
Group: core-security
Attachment #643070 - Flags: review?(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.
Attachment #643070 - Flags: review?(jwatt) → review+
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
OS: Mac OS X → All
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/f86536e718e8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
I don't see any crashes like the stack trace here, so it probably isn't worth backporting.
You need to log in before you can comment on or make changes to this bug.