If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Use-after-poison [@ mozilla::FontFamilyList::FontFamilyList] with unicode-bidi: bidi-override

RESOLVED FIXED in Firefox 35

Status

()

Core
CSS Parsing and Computation
--
critical
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: Jesse Ruderman, Assigned: heycam)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla35
x86_64
All
crash, csectype-framepoisoning, sec-other, testcase
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox35 fixed, firefox-esr31 unaffected)

Details

(Whiteboard: [adv-main35+], crash signature)

Attachments

(5 attachments)

(Reporter)

Description

3 years ago
Created attachment 8494288 [details]
testcase (crashes Firefox)
(Reporter)

Comment 1

3 years ago
Created attachment 8494292 [details]
stack (lldb)
(Reporter)

Comment 2

3 years ago
Created attachment 8494293 [details]
stack (ASan)
In nsLayoutUtils::GetFontMetricsForStyleContext:
3414      nsFont font = aStyleContext->StyleFont()->mFont;

aStyleContext looks fine, the StyleFont() points to a destroyed object,
i.e. memory still allocated but poisoned in the pres shell arena.

Maybe the same underlying problem as bug 1070759?
Component: Layout: Text → CSS Parsing and Computation
Keywords: csectype-framepoisoning, sec-other
OS: Mac OS X → All
(Assignee)

Comment 4

3 years ago
Might be different.  With this bug, if I enable the more expensive style struct destruction checking http://hg.mozilla.org/mozilla-central/file/5e704397529b/layout/style/nsStyleContext.cpp#l89 then it triggers for me:

style struct 0x625000771300 found on style context 0x62500077b7b8
  in file:///tmp/test2.html
Assertion failure: false (destroying Font style struct still present in style context tree), at ./nsStyleStructList.h:44

but not in bug 1070759.

So it's likely this bug is a regression for bug 931668.
(Assignee)

Comment 5

3 years ago
Created attachment 8494992 [details]
restyle logging output

Using the restyle logging patches from bug 979133 and bug 1072724 I get this output.  Pretty sure we shouldn't be swapping structs for the same-style continuations.
Assignee: nobody → cam
Status: NEW → ASSIGNED
(Assignee)

Comment 6

3 years ago
Looks like this is a change I was going to have in bug 931668, but when splitting up my patch queue it ended up in the bug 979133 patch (see bug 979133 comment 15's mention of copyFromContinuations).
(Assignee)

Comment 7

3 years ago
Created attachment 8494997 [details] [diff] [review]
patch
Attachment #8494997 - Flags: review?(dbaron)
Attachment #8494997 - Flags: review?(dbaron) → review+
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/64136343bb4b
https://hg.mozilla.org/mozilla-central/rev/64136343bb4b
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox35: --- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
status-firefox-esr31: --- → unaffected
Whiteboard: [adv-main35+]

Updated

2 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.