diff options
author | dsinclair <dsinclair@chromium.org> | 2016-08-23 11:39:23 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-08-23 11:39:23 -0700 |
commit | 837735660808d52580703183ae24a3c7c7b05c7d (patch) | |
tree | c0a607bfd0491fbf18988bf4dbe9f034571bfdf7 /xfa/fxfa/include | |
parent | c38de1116bbee807e4461fe8a08e4c152c0fce15 (diff) | |
download | pdfium-837735660808d52580703183ae24a3c7c7b05c7d.tar.xz |
[XFA] Force destruction order of font managers.chromium/2838
The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.
The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.
BUG=chromium:637546
Review-Url: https://codereview.chromium.org/2259823004
Diffstat (limited to 'xfa/fxfa/include')
-rw-r--r-- | xfa/fxfa/include/xfa_ffapp.h | 15 |
1 files changed, 14 insertions, 1 deletions
diff --git a/xfa/fxfa/include/xfa_ffapp.h b/xfa/fxfa/include/xfa_ffapp.h index bc0d6dfb38..90bfcc0240 100644 --- a/xfa/fxfa/include/xfa_ffapp.h +++ b/xfa/fxfa/include/xfa_ffapp.h @@ -63,13 +63,26 @@ class CXFA_FFApp { protected: std::unique_ptr<CXFA_FFDocHandler> m_pDocHandler; IXFA_AppProvider* const m_pProvider; + + // The fonts stored in the font manager may have been created by the default + // font manager. The GEFont::LoadFont call takes the manager as a param and + // stores it internally. When you destroy the GEFont it tries to unregister + // from the font manager and if the default font manager was destroyed first + // get get a use-after-free. The m_pFWLTheme can try to cleanup a GEFont + // when it frees, so make sure it gets cleaned up first. That requires + // m_pFWLApp to be cleaned up as well. + // + // TODO(dsinclair): The GEFont should have the FontMgr as the pointer instead + // of the DEFFontMgr so this goes away. Bug 561. + std::unique_ptr<IFGAS_FontMgr> m_pFDEFontMgr; std::unique_ptr<CXFA_FontMgr> m_pFontMgr; + #if _FXM_PLATFORM_ != _FXM_PLATFORM_WINDOWS_ std::unique_ptr<CFX_FontSourceEnum_File> m_pFontSource; #endif std::unique_ptr<CXFA_FWLAdapterWidgetMgr> m_pAdapterWidgetMgr; CFWL_WidgetMgrDelegate* m_pWidgetMgrDelegate; // not owned. - std::unique_ptr<IFGAS_FontMgr> m_pFDEFontMgr; + // |m_pFWLApp| has to be released first, then |m_pFWLTheme| since the former // may refers to theme manager and the latter refers to font manager. std::unique_ptr<CXFA_FWLTheme> m_pFWLTheme; |