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

Michael Stahl mstahl at redhat.com
Tue Jul 4 10:29:51 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 d95ac92b8530b0a48b468862b722daa88215228e
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/39102
    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 cf10f8805a96..514e4a43eae5 100644
--- a/sw/source/core/layout/flowfrm.cxx
+++ b/sw/source/core/layout/flowfrm.cxx
@@ -402,8 +402,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 032f0cf9a304..54cb686637e6 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