- Revision
- 243828
- Author
- mmaxfi...@apple.com
- Date
- 2019-04-03 14:46:55 -0700 (Wed, 03 Apr 2019)
Log Message
Documents can be destroyed before their CSSFontFaceSet is destroyed
https://bugs.webkit.org/show_bug.cgi?id=195830
Reviewed by Darin Adler.
Source/WebCore:
CSSFontFaceSet has a raw pointer to its owning document. JS can keep the CSSFontFaceSet alive (by using FontFaceSet)
and can destroy the document at any time. When the document is destroyed, the link between the two objects needs to
be severed.
Test: fast/text/font-face-set-destroy-document.html
* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::CSSFontFace):
* css/CSSFontFace.h:
* css/CSSFontFaceSet.cpp:
(WebCore::CSSFontFaceSet::CSSFontFaceSet):
(WebCore::CSSFontFaceSet::ensureLocalFontFacesForFamilyRegistered):
* css/CSSFontFaceSet.h:
* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::CSSFontSelector):
(WebCore::CSSFontSelector::addFontFaceRule):
* css/CSSFontSelector.h:
* css/FontFace.cpp:
(WebCore::FontFace::FontFace):
LayoutTests:
* fast/text/font-face-set-destroy-document-expected.html: Added.
* fast/text/font-face-set-destroy-document.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (243827 => 243828)
--- trunk/LayoutTests/ChangeLog 2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/LayoutTests/ChangeLog 2019-04-03 21:46:55 UTC (rev 243828)
@@ -1,3 +1,13 @@
+2019-04-03 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Documents can be destroyed before their CSSFontFaceSet is destroyed
+ https://bugs.webkit.org/show_bug.cgi?id=195830
+
+ Reviewed by Darin Adler.
+
+ * fast/text/font-face-set-destroy-document-expected.html: Added.
+ * fast/text/font-face-set-destroy-document.html: Added.
+
2019-04-03 Shawn Roberts <srobe...@apple.com>
http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-user-interaction-but-access-from-wrong-frame.html is a flaky timeout
Added: trunk/LayoutTests/fast/text/font-face-set-destroy-document-expected.html (0 => 243828)
--- trunk/LayoutTests/fast/text/font-face-set-destroy-document-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/font-face-set-destroy-document-expected.html 2019-04-03 21:46:55 UTC (rev 243828)
@@ -0,0 +1,3 @@
+<html>
+This test makes sure FontFaceSets don't access deleted documents. The test passes if there is no crash when running under ASan.
+</html>
Added: trunk/LayoutTests/fast/text/font-face-set-destroy-document.html (0 => 243828)
--- trunk/LayoutTests/fast/text/font-face-set-destroy-document.html (rev 0)
+++ trunk/LayoutTests/fast/text/font-face-set-destroy-document.html 2019-04-03 21:46:55 UTC (rev 243828)
@@ -0,0 +1,15 @@
+<html>
+This test makes sure FontFaceSets don't access deleted documents. The test passes if there is no crash when running under ASan.
+<script>
+
+d = document.implementation.createDocument(null, '');
+f = new FontFace('f', 'local(F)', {});
+ffs = d.fonts;
+delete d;
+// gc();
+GCController.collect();
+
+// trigger use after free
+ffs.add(f);
+</script>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (243827 => 243828)
--- trunk/Source/WebCore/ChangeLog 2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/ChangeLog 2019-04-03 21:46:55 UTC (rev 243828)
@@ -1,3 +1,30 @@
+2019-04-03 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Documents can be destroyed before their CSSFontFaceSet is destroyed
+ https://bugs.webkit.org/show_bug.cgi?id=195830
+
+ Reviewed by Darin Adler.
+
+ CSSFontFaceSet has a raw pointer to its owning document. JS can keep the CSSFontFaceSet alive (by using FontFaceSet)
+ and can destroy the document at any time. When the document is destroyed, the link between the two objects needs to
+ be severed.
+
+ Test: fast/text/font-face-set-destroy-document.html
+
+ * css/CSSFontFace.cpp:
+ (WebCore::CSSFontFace::CSSFontFace):
+ * css/CSSFontFace.h:
+ * css/CSSFontFaceSet.cpp:
+ (WebCore::CSSFontFaceSet::CSSFontFaceSet):
+ (WebCore::CSSFontFaceSet::ensureLocalFontFacesForFamilyRegistered):
+ * css/CSSFontFaceSet.h:
+ * css/CSSFontSelector.cpp:
+ (WebCore::CSSFontSelector::CSSFontSelector):
+ (WebCore::CSSFontSelector::addFontFaceRule):
+ * css/CSSFontSelector.h:
+ * css/FontFace.cpp:
+ (WebCore::FontFace::FontFace):
+
2019-04-03 Sihui Liu <sihui_...@apple.com>
Follow up fix for r243807: Use MarkedArgumentBuffer instead of Vector for JSValue
Modified: trunk/Source/WebCore/css/CSSFontFace.h (243827 => 243828)
--- trunk/Source/WebCore/css/CSSFontFace.h 2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/css/CSSFontFace.h 2019-04-03 21:46:55 UTC (rev 243828)
@@ -189,7 +189,7 @@
FontLoadingBehavior m_loadingBehavior { FontLoadingBehavior::Auto };
Vector<std::unique_ptr<CSSFontFaceSource>, 0, CrashOnOverflow, 0> m_sources;
- RefPtr<CSSFontSelector> m_fontSelector;
+ RefPtr<CSSFontSelector> m_fontSelector; // FIXME: https://bugs.webkit.org/show_bug.cgi?id=196437 There's a retain cycle: CSSFontSelector -> CSSFontFaceSet -> CSSFontFace -> CSSFontSelector
RefPtr<StyleRuleFontFace> m_cssConnection;
HashSet<Client*> m_clients;
WeakPtr<FontFace> m_wrapper;
Modified: trunk/Source/WebCore/css/CSSFontFaceSet.cpp (243827 => 243828)
--- trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2019-04-03 21:46:55 UTC (rev 243828)
@@ -42,7 +42,7 @@
namespace WebCore {
CSSFontFaceSet::CSSFontFaceSet(CSSFontSelector* owningFontSelector)
- : m_owningFontSelector(owningFontSelector)
+ : m_owningFontSelector(makeWeakPtr(owningFontSelector))
{
}
@@ -113,7 +113,7 @@
Vector<Ref<CSSFontFace>> faces;
for (auto item : capabilities) {
- Ref<CSSFontFace> face = CSSFontFace::create(m_owningFontSelector, nullptr, nullptr, true);
+ Ref<CSSFontFace> face = CSSFontFace::create(m_owningFontSelector.get(), nullptr, nullptr, true);
Ref<CSSValueList> familyList = CSSValueList::createCommaSeparated();
familyList->append(CSSValuePool::singleton().createFontFamilyValue(familyName));
Modified: trunk/Source/WebCore/css/CSSFontFaceSet.h (243827 => 243828)
--- trunk/Source/WebCore/css/CSSFontFaceSet.h 2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.h 2019-04-03 21:46:55 UTC (rev 243828)
@@ -120,7 +120,7 @@
size_t m_facesPartitionIndex { 0 }; // All entries in m_faces before this index are CSS-connected.
Status m_status { Status::Loaded };
HashSet<CSSFontFaceSetClient*> m_clients;
- CSSFontSelector* m_owningFontSelector;
+ WeakPtr<CSSFontSelector> m_owningFontSelector;
unsigned m_activeCount { 0 };
};
Modified: trunk/Source/WebCore/css/CSSFontSelector.h (243827 => 243828)
--- trunk/Source/WebCore/css/CSSFontSelector.h 2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/css/CSSFontSelector.h 2019-04-03 21:46:55 UTC (rev 243828)
@@ -47,7 +47,7 @@
class Document;
class StyleRuleFontFace;
-class CSSFontSelector final : public FontSelector, public CSSFontFaceSetClient {
+class CSSFontSelector final : public FontSelector, public CSSFontFaceSetClient, public CanMakeWeakPtr<CSSFontSelector> {
public:
static Ref<CSSFontSelector> create(Document& document)
{
