[Libreoffice-commits] core.git: Branch 'libreoffice-5-3' - sw/source

Michael Stahl mstahl at redhat.com
Tue Jul 11 15:41:36 UTC 2017


 sw/source/core/layout/flowfrm.cxx               |   14 ++++++++++--
 sw/source/core/layout/objectformattertxtfrm.cxx |   27 +++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 3 deletions(-)

New commits:
commit f8b32e4388cfc9cfcf1acebc82d023a8d3783463
Author: Michael Stahl <mstahl at redhat.com>
Date:   Wed Jun 21 12:26:19 2017 +0200

    tdf#101821 sw: fix layout footnote use-after-free
    
    After inserting a header in the bugdoc, during SwFrame::Calc of a
    SwTextFrame that is in a footnote, it decides move forward to the next
    page.  This deletes the SwFootnoteFrame and SwFootnoteContFrame that
    lcl_FormatContentOfLayoutFrame() are iterating over.
    
    For want of a more elegant solution, use a big hammer to prevent the
    problem and try to clean up so that no empty SwFootnoteFrame and
    SwFootnoteContFrame remain (as that is known to crash in other places,
    see commit c9fb347642729017ad0c613fe26310befd021db8)
    
    Invalid read of size 8
       at 0x414E1F96: SwFrame::GetNext() (frame.hxx:485)
       by 0x41AFDD07: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:646)
       by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
       by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
       by 0x41AFDEA7: SwObjectFormatterTextFrame::FormatAnchorFrameAndItsPrevs(SwTextFrame&) (objectformattertxtfrm.cxx:696)
       by 0x41AAA680: SwFlyAtContentFrame::MakeAll(OutputDevice*) (flycnt.cxx:415)
       by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
       by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
       by 0x41AA8927: SwFlyFrame::Calc(OutputDevice*) const (fly.cxx:2559)
       by 0x41ADB36B: SwLayAction::FormatLayoutFly(SwFlyFrame*) (layact.cxx:1414)
       by 0x41AF9658: SwObjectFormatter::FormatObj_(SwAnchoredObject&) (objectformatter.cxx:321)
       by 0x41AFCB6E: SwObjectFormatterTextFrame::DoFormatObj(SwAnchoredObject&, bool) (objectformattertxtfrm.cxx:126)
       by 0x41AF9A6A: SwObjectFormatter::FormatObjsAtFrame_(SwTextFrame*) (objectformatter.cxx:443)
       by 0x41AFD275: SwObjectFormatterTextFrame::DoFormatObjs() (objectformattertxtfrm.cxx:328)
       by 0x41AF924A: SwObjectFormatter::FormatObjsAtFrame(SwFrame&, SwPageFrame const&, SwLayAction*) (objectformatter.cxx:191)
       by 0x41ADC213: SwLayAction::FormatContent(SwPageFrame const*) (layact.cxx:1633)
       by 0x41AD88DE: SwLayAction::InternalAction(OutputDevice*) (layact.cxx:760)
       by 0x41AD7080: SwLayAction::Action(OutputDevice*) (layact.cxx:351)
       by 0x41ADE32E: SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) (layact.cxx:2133)
       by 0x41FFC97E: SwViewShell::LayoutIdle() (viewsh.cxx:711)
     Address 0x505541a8 is 72 bytes inside a block of size 272 free'd
       at 0x4C2F21A: operator delete(void*) (vg_replace_malloc.c:576)
       by 0x41AD371A: SwFootnoteFrame::~SwFootnoteFrame() (ftnfrm.hxx:52)
       by 0x41B5B74C: SwFrame::DestroyFrame(SwFrame*) (ssfrm.cxx:391)
       by 0x41A97294: SwFlowFrame::CutTree(SwFrame*) (flowfrm.cxx:406)
       by 0x41A979AE: SwFlowFrame::MoveSubTree(SwLayoutFrame*, SwFrame*) (flowfrm.cxx:592)
       by 0x41ACFB69: SwContentFrame::MoveFootnoteCntFwd(bool, SwFootnoteBossFrame*) (ftnfrm.cxx:2756)
       by 0x41A9B78E: SwFlowFrame::MoveFwd(bool, bool, bool) (flowfrm.cxx:1813)
       by 0x41A85864: SwContentFrame::MakeAll(OutputDevice*) (calcmove.cxx:1681)
       by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
       by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
       by 0x41AFDCFB: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:644)
       by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
       by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
       by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
       by 0x41AFDEA7: SwObjectFormatterTextFrame::FormatAnchorFrameAndItsPrevs(SwTextFrame&) (objectformattertxtfrm.cxx:696)
       by 0x41AAA680: SwFlyAtContentFrame::MakeAll(OutputDevice*) (flycnt.cxx:415)
       by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
       by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
       by 0x41AA8927: SwFlyFrame::Calc(OutputDevice*) const (fly.cxx:2559)
       by 0x41ADB36B: SwLayAction::FormatLayoutFly(SwFlyFrame*) (layact.cxx:1414)
       by 0x41AF9658: SwObjectFormatter::FormatObj_(SwAnchoredObject&) (objectformatter.cxx:321)
       by 0x41AFCB6E: SwObjectFormatterTextFrame::DoFormatObj(SwAnchoredObject&, bool) (objectformattertxtfrm.cxx:126)
       by 0x41AF9A6A: SwObjectFormatter::FormatObjsAtFrame_(SwTextFrame*) (objectformatter.cxx:443)
       by 0x41AFD275: SwObjectFormatterTextFrame::DoFormatObjs() (objectformattertxtfrm.cxx:328)
       by 0x41AF924A: SwObjectFormatter::FormatObjsAtFrame(SwFrame&, SwPageFrame const&, SwLayAction*) (objectformatter.cxx:191)
       by 0x41ADC213: SwLayAction::FormatContent(SwPageFrame const*) (layact.cxx:1633)
       by 0x41AD88DE: SwLayAction::InternalAction(OutputDevice*) (layact.cxx:760)
       by 0x41AD7080: SwLayAction::Action(OutputDevice*) (layact.cxx:351)
       by 0x41ADE32E: SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) (layact.cxx:2133)
       by 0x41FFC97E: SwViewShell::LayoutIdle() (viewsh.cxx:711)
    
    Change-Id: I656cc2303eeccd5eef68ad3b8e81bb0fd47b94fb
    (cherry picked from commit 9dcb767c5bdccdf6606240afd6aa2c6bd3dcc9f4)
    Reviewed-on: https://gerrit.libreoffice.org/39105
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>

diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx
index c2ccaddaf66c..14a6ad91eac9 100644
--- a/sw/source/core/layout/flowfrm.cxx
+++ b/sw/source/core/layout/flowfrm.cxx
@@ -401,8 +401,18 @@ SwLayoutFrame *SwFlowFrame::CutTree( SwFrame *pStart )
         if ( !pLay->Lower() && !pLay->IsColLocked() &&
              !static_cast<SwFootnoteFrame*>(pLay)->IsBackMoveLocked() )
         {
-            pLay->Cut();
-            SwFrame::DestroyFrame(pLay);
+            // tdf#101821 don't delete it while iterating over it
+            if (!pLay->IsDeleteForbidden())
+            {
+                pLay->Cut();
+                SwFrame::DestroyFrame(pLay);
+            }
+            // else: assume there is code on the stack to clean up empty
+            // footnote frames
+            // (don't go into the else branch below, it produces a disconnected
+            // footnote with null upper that can be returned by
+            // SwFootnoteBossFrame::FindFootnote() causing null pointer deref
+            // in SwTextFrame::ConnectFootnote()
         }
         else
         {
diff --git a/sw/source/core/layout/objectformattertxtfrm.cxx b/sw/source/core/layout/objectformattertxtfrm.cxx
index 16d3ed4fc764..d77dc89ae996 100644
--- a/sw/source/core/layout/objectformattertxtfrm.cxx
+++ b/sw/source/core/layout/objectformattertxtfrm.cxx
@@ -29,6 +29,7 @@
 #include <fmtwrapinfluenceonobjpos.hxx>
 #include <fmtfollowtextflow.hxx>
 #include <layact.hxx>
+#include <ftnfrm.hxx>
 
 using namespace ::com::sun::star;
 
@@ -638,12 +639,36 @@ static void lcl_FormatContentOfLayoutFrame( SwLayoutFrame* pLayFrame,
             break;
         }
         if ( pLowerFrame->IsLayoutFrame() )
+        {
+            SwFrameDeleteGuard aCrudeHack(pLowerFrame); // ??? any issue setting this for non-footnote frames?
             lcl_FormatContentOfLayoutFrame( static_cast<SwLayoutFrame*>(pLowerFrame),
                                         pLastLowerFrame );
+        }
         else
             pLowerFrame->Calc(pLowerFrame->getRootFrame()->GetCurrShell()->GetOut());
 
-        pLowerFrame = pLowerFrame->GetNext();
+        // Calc on a SwTextFrame in a footnote can move it to the next page -
+        // deletion of the SwFootnoteFrame was disabled with SwFrameDeleteGuard
+        // but now we have to clean up empty footnote frames to prevent crashes.
+        // Note: check it at this level, not lower: both container and footnote
+        // can be deleted at the same time!
+        SwFrame *const pNext = pLowerFrame->GetNext();
+        if (pLowerFrame->IsFootnoteContFrame())
+        {
+            for (SwFrame * pFootnote = pLowerFrame->GetLower(); pFootnote; )
+            {
+                assert(pFootnote->IsFootnoteFrame());
+                SwFrame *const pNextNote = pFootnote->GetNext();
+                if (!pFootnote->GetLower() && !pFootnote->IsColLocked() &&
+                    !static_cast<SwFootnoteFrame*>(pFootnote)->IsBackMoveLocked())
+                {
+                    pFootnote->Cut();
+                    SwFrame::DestroyFrame(pFootnote);
+                }
+                pFootnote = pNextNote;
+            }
+        }
+        pLowerFrame = pNext;
     }
 }
 


More information about the Libreoffice-commits mailing list