[Libreoffice-commits] core.git: sw/source

[Michael Stahl (via logerrit)]

 sw/source/core/layout/calcmove.cxx |   30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

New commits:
commit a23661bfe02d81bd9c1f1e04c59384d19cc61726
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Fri Jun 21 16:48:08 2019 +0200
Commit:     Michael Stahl <Michael.Stahl at cib.de>
CommitDate: Mon Jun 24 11:44:40 2019 +0200

    tdf#125741 sw: avoid joining table frames in SwFrame::PrepareCursor()
    
    The problem is that the PrepareCursor() is called on a frame in a nested
    table, and the inner table is joined by its predecessor...
    
    0x6120010a3348 is located 136 bytes inside of 296-byte region [0x6120010a32c0,0x6120010a33e8)
    freed by thread T0 here:
        #1 SwTabFrame::~SwTabFrame() sw/source/core/layout/tabfrm.cxx:143:1
        #2 SwFrame::DestroyFrame(SwFrame*) sw/source/core/layout/ssfrm.cxx:389:9
        #3 SwTabFrame::Join() sw/source/core/layout/tabfrm.cxx:1379:9
        #4 SwTabFrame::MakeAll(OutputDevice*) sw/source/core/layout/tabfrm.cxx:1870:9
        #5 SwFrame::PrepareMake(OutputDevice*) sw/source/core/layout/calcmove.cxx:364:5
        #6 SwFrame::Calc(OutputDevice*) const sw/source/core/layout/trvlfrm.cxx:1791:37
        #7 lcl_InnerCalcLayout(SwFrame*, long, bool) sw/source/core/layout/tabfrm.cxx:1584:21
        #8 lcl_InnerCalcLayout(SwFrame*, long, bool) sw/source/core/layout/tabfrm.cxx:1586:25
        #9 lcl_InnerCalcLayout(SwFrame*, long, bool) sw/source/core/layout/tabfrm.cxx:1586:25
        #10 lcl_RecalcRow(SwRowFrame*, long) sw/source/core/layout/tabfrm.cxx:1621:16
        #11 SwTabFrame::MakeAll(OutputDevice*) sw/source/core/layout/tabfrm.cxx:2410:21
        #12 SwFrame::PrepareMake(OutputDevice*) sw/source/core/layout/calcmove.cxx:364:5
        #13 SwFrame::Calc(OutputDevice*) const sw/source/core/layout/trvlfrm.cxx:1791:37
        #14 SwFrame::PrepareCursor() sw/source/core/layout/calcmove.cxx:481:5
        #15 SwFrame::PrepareCursor() sw/source/core/layout/calcmove.cxx:397:21
        #16 SwFrame::PrepareCursor() sw/source/core/layout/calcmove.cxx:397:21
        #17 SwFrame::PrepareCursor() sw/source/core/layout/calcmove.cxx:397:21
        #18 SwFrame::PrepareCursor() sw/source/core/layout/calcmove.cxx:397:21
        #19 SwFrame::PrepareCursor() sw/source/core/layout/calcmove.cxx:397:21
        #20 SwFrame::PrepareCursor() sw/source/core/layout/calcmove.cxx:397:21
        #21 SwCursorShell::UpdateCursor(unsigned short, bool) sw/source/core/crsr/crsrsh.cxx:1851:25
        #22 SwCursorShell::EndAction(bool, bool) sw/source/core/crsr/crsrsh.cxx:269:5
        #23 SwCursorShell::EndCursorMove(bool) sw/source/core/crsr/crsrsh.cxx:305:5
        #24 SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) sw/source/core/layout/layact.cxx:2238:35
    
    After the paste, the formatting goes only until
    SwLayAction::IsShortCut() exits early because the previous visible page
    is formatted - but the cursor position was moved downward and is no
    longer visible, so we get an idle-layout later where UpdateCursor()
    triggers more formatting...
    
    So prevent the use-after-free with some more guards, which isn't the
    most elegant solution, but with 4 levels of nested tables elegant
    solutions are in short supply...
    
    (apparently a regression from the SwFlowFrame::MoveBwd() change in
     18765b9fa739337d2d891513f6e2fb7c3ce23b50)
    
    Change-Id: Ie166d3b58fe84c3e4808b52202802a471fa81026
    Reviewed-on: https://gerrit.libreoffice.org/74518
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <Michael.Stahl at cib.de>

diff --git a/sw/source/core/layout/calcmove.cxx b/sw/source/core/layout/calcmove.cxx
index fc4bc4567079..874168d40fcb 100644
--- a/sw/source/core/layout/calcmove.cxx
+++ b/sw/source/core/layout/calcmove.cxx
@@ -394,32 +394,37 @@ void SwFrame::PrepareCursor()
     StackHack aHack;
     if( GetUpper() && !GetUpper()->IsSctFrame() )
     {
-        GetUpper()->PrepareCursor();
-        GetUpper()->Calc(getRootFrame()->GetCurrShell() ? getRootFrame()->GetCurrShell()->GetOut() : nullptr);
-
-        OSL_ENSURE( GetUpper(), ":-( Layout unstable (Upper gone)." );
-        if ( !GetUpper() )
-            return;
-
         const bool bCnt = IsContentFrame();
         const bool bTab = IsTabFrame();
         bool bNoSect = IsInSct();
 
-        bool bOldTabLock = false, bFoll;
+        boost::optional<FlowFrameJoinLockGuard> tabGuard;
+        boost::optional<SwFrameDeleteGuard> rowGuard;
         SwFlowFrame* pThis = bCnt ? static_cast<SwContentFrame*>(this) : nullptr;
 
         if ( bTab )
         {
-            bOldTabLock = static_cast<SwTabFrame*>(this)->IsJoinLocked();
-            ::PrepareLock( static_cast<SwTabFrame*>(this) );
+            tabGuard.emplace(static_cast<SwTabFrame*>(this)); // tdf#125741
             pThis = static_cast<SwTabFrame*>(this);
         }
+        else if (IsRowFrame())
+        {
+            rowGuard.emplace(this); // tdf#125741 keep this alive
+        }
         else if( IsSctFrame() )
         {
             pThis = static_cast<SwSectionFrame*>(this);
             bNoSect = false;
         }
-        bFoll = pThis && pThis->IsFollow();
+
+        GetUpper()->PrepareCursor();
+        GetUpper()->Calc(getRootFrame()->GetCurrShell() ? getRootFrame()->GetCurrShell()->GetOut() : nullptr);
+
+        OSL_ENSURE( GetUpper(), ":-( Layout unstable (Upper gone)." );
+        if ( !GetUpper() )
+            return;
+
+        bool const bFoll = pThis && pThis->IsFollow();
 
         SwFrame *pFrame = GetUpper()->Lower();
         while ( pFrame != this )
@@ -474,9 +479,6 @@ void SwFrame::PrepareCursor()
         GetUpper()->Calc(getRootFrame()->GetCurrShell()->GetOut());
 
         OSL_ENSURE( GetUpper(), "Layout unstable (Upper gone III)." );
-
-        if ( bTab && !bOldTabLock )
-            ::PrepareUnlock( static_cast<SwTabFrame*>(this) );
     }
     Calc(getRootFrame()->GetCurrShell() ? getRootFrame()->GetCurrShell()->GetOut() : nullptr);
 }