The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla17

Status

()

Core
SVG
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: mccr8)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
mozilla17
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 638390 [details]
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.
(Reporter)

Comment 1

5 years ago
Created attachment 638391 [details]
stack trace
(Reporter)

Comment 2

5 years ago
Might be debug-only.
Crash Signature: [@ mozilla::DOMSVGTransformList::IsAnimValList] → [@ mozilla::DOMSVGTransformList::IsAnimValList] [@ mozilla::DOMSVGTransformList::cycleCollection::UnlinkImpl]
(Assignee)

Updated

5 years ago
Assignee: nobody → continuation
(Assignee)

Comment 3

5 years ago
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
(Assignee)

Comment 4

5 years ago
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
(Assignee)

Comment 5

5 years ago
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.
(Assignee)

Updated

5 years ago
Group: core-security
(Assignee)

Comment 6

5 years ago
Created attachment 643070 [details] [diff] [review]
null check in case of double unlink
(Assignee)

Updated

5 years ago
Attachment #643070 - Flags: review?(jwatt)
(Assignee)

Comment 7

5 years ago
Try run looked good.

https://tbpl.mozilla.org/?tree=Try&rev=9a8ad9fdc466
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+
(Assignee)

Comment 9

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
(Assignee)

Comment 11

5 years ago
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.