Bug 119224 - Crash scrolling formatted 2007 DOCX with revision, comments and images (SfxListener::~SfxListener() EXCEPTION_ACCESS_VIOLATION_WRITE) ( steps in comment 8 )
Summary: Crash scrolling formatted 2007 DOCX with revision, comments and images (SfxLi...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.4 all versions
Hardware: All All
: medium major
Assignee: Not Assigned
URL:
Whiteboard: target:6.2.0 target:6.1.1 target:7.6.0
Keywords: filter:docx, haveBacktrace
Depends on:
Blocks:
 
Reported: 2018-08-12 06:46 UTC by philchan2008
Modified: 2023-01-18 20:19 UTC (History)
4 users (show)

See Also:
Crash report or crash signature: ["SfxListener::~SfxListener()","SwShellCursor::FillRects()"]


Attachments
Sample docx which can crash the LO but open normal in MSWord2016/2007 (160.20 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2018-08-12 06:47 UTC, philchan2008
Details
bt with debug symbols (4.28 KB, text/plain)
2018-08-12 09:58 UTC, Julien Nabet
Details
Valgrind trace (43.94 KB, application/x-bzip)
2018-08-12 11:00 UTC, Julien Nabet
Details
Sample docx saved in MSO.pdf (392.44 KB, application/pdf)
2018-08-15 16:47 UTC, Timur
Details
cut down valgrind log to first relevent part (2.64 KB, text/plain)
2018-08-16 13:26 UTC, Caolán McNamara
Details

Note You need to log in before you can comment on or make changes to this bug.
Description philchan2008 2018-08-12 06:46:23 UTC
Description:
When LibreOffice try to open the .docx from MSWord which contains table and image change tracks and comments (from MSWord 2016/2007 but not all .docx will crash, only some with many complicated changes), then it will crash the Libreoffice during document loading.

Related crash report: http://crashreport.libreoffice.org/stats/crash_details/0ad37c47-8291-4201-b0a9-2d21d6e3b0ad#allthreads


Steps to Reproduce:
1. Open the provided .docx (which is able to open MSWord)
2. Crash during loading

Actual Results:
Crash, then exit.

Expected Results:
Able to read and modify the docx file.


Reproducible: Always


User Profile Reset: Yes


OpenGL enabled: Yes

Additional Info:
Comment 1 philchan2008 2018-08-12 06:47:58 UTC
Created attachment 144120 [details]
Sample docx which can crash the LO but open normal in MSWord2016/2007
Comment 2 philchan2008 2018-08-12 06:49:38 UTC
Open the docx then scroll down will crash the LO.
Comment 3 raal 2018-08-12 08:33:50 UTC
I can confirm with Version: 6.2.0.0.alpha0+
Build ID: ea59fc4831b9d2430de51faa8c3e0a24e6d90cd1
CPU threads: 4; OS: Linux 4.4; UI render: default; VCL: gtk3; 
and 4.1
Comment 4 Julien Nabet 2018-08-12 09:58:59 UTC
Created attachment 144122 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated yesterday, I got a crash but different from the crashreport.

I also noticed logs like:
warn:sw:6045:6045:sw/source/core/doc/DocumentRedlineManager.cxx:100: redline table corrupted: overlapping redlines
warn:legacy.osl:6045:6045:sw/source/core/text/itrform2.cxx:403: SwTextFormatter::BuildPortions: bad length in info
warn:legacy.osl:6045:6045:sw/source/core/layout/flylay.cxx:706: <SwFlyFreeFrame::CheckClip(..)> - fly frame has negative height now.
warn:legacy.osl:6045:6045:svx/source/sdr/contact/viewcontactofsdrpathobj.cxx:56: PolyPolygon object without geometry detected, this should not be created (!)
Comment 5 Julien Nabet 2018-08-12 11:00:31 UTC
Created attachment 144126 [details]
Valgrind trace
Comment 6 Xisco Faulí 2018-08-13 17:58:51 UTC
it crashes for me in

Version: 6.2.0.0.alpha0+
Build ID: 53eda574a61396b6765cd1cb0ac9804c754ac4c1
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: threaded

after scrolling down to page 3-4...
Comment 7 Timur 2018-08-15 16:47:31 UTC
Created attachment 144203 [details]
Sample docx saved in MSO.pdf

raal marked as 4.1, but I don't confirm that in Win, just from 4.4
I'll set regression until explained otherwise. 
I also don't repro with docx saved in MSO, looks like this is 2007 docx.
Comment 8 Xisco Faulí 2018-08-15 17:21:18 UTC
Some more info...
Previous to 4.4, it crashes for me at import time. This got fixed in https://cgit.freedesktop.org/libreoffice/core/commit/?id=8fcd4bf323bc3390e366229d549641444b5a3e9a by Caolán and from this point up to master, the files loads fine, however it crashes scrolling it.

@Caolán, I thought you could be interested in this issue...

Steps to reproduce:
1. Load the file attached
2. Wait until the file is completely loaded ( it's recalculated to 8 pages )
3. Scroll down either with the mouse wheel or with the page down key...
Comment 9 Xisco Faulí 2018-08-15 17:23:51 UTC
I get a different crashreport following the steps from comment 8:
http://crashreport.libreoffice.org/stats/crash_details/64bcb0c6-598c-4077-a3b9-63caa6ff88b6, which seems to match Julien's backtrace...
Comment 10 Caolán McNamara 2018-08-16 13:26:47 UTC
Created attachment 144238 [details]
cut down valgrind log to first relevent part
Comment 11 Caolán McNamara 2018-08-16 13:41:01 UTC
I get the same bt as Julien and looks to me that pEndFrame is assigned at the top and the GetFormatted causes it to get deleted so using it later on crashes. Seems that pStartFrame and pEndFrame are expected to exist for the duration of the whole function. Sticking the SwFrameDeleteGuard hammer in there for pStartFrame and pEndFrame makes it not crash for me, and nothing output under valgrind https://gerrit.libreoffice.org/#/c/59176/
Comment 12 Julien Nabet 2018-08-16 13:44:44 UTC
(In reply to Caolán McNamara from comment #11)
Just for curiosity "Sticking the SwFrameDeleteGuard hammer" means it's more a bandaid fix waiting for a right fix which needs far more time (eg: need of redesign frames management)?
Comment 13 Caolán McNamara 2018-08-16 14:44:47 UTC
(In reply to Julien Nabet from comment #12)
> (In reply to Caolán McNamara from comment #11)
> Just for curiosity "Sticking the SwFrameDeleteGuard hammer" means it's more
> a bandaid fix waiting for a right fix which needs far more time (eg: need of
> redesign frames management)?

I think that's a fair representation of the SwFrameDeleteGuard :-) It tags a frame as one that shouldn't be deleted, and in this function is seems straight forward that the assumption in this method is that those start/end frames are expected to not get removed during this function. Writer layout is very convoluted and I wouldn't even hazard a guess as to what might be a perfect fix.
Comment 14 Commit Notification 2018-08-16 15:53:24 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=00faf6dd9957890634db94cfc6b8a1108a62a2d2

tdf#119224 start and end are expected to exist for the scope of this function

It will be available in 6.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Caolán McNamara 2018-08-16 19:04:50 UTC
testing appreciated to see if this helps, it may well be the case that there is another crash elsewhere with this document.
Comment 16 philchan2008 2018-08-17 03:16:55 UTC
Problem solved, I installed and tested with both the test case file and original complex file on Win10-64bit platform, there is NO crash any more.

ref. link: https://dev-builds.libreoffice.org/daily/master/Win-x86_64@42/2018-08-17_00.41.06/
Comment 17 Xisco Faulí 2018-08-17 07:11:56 UTC
it no longer crashes in

Version: 6.2.0.0.alpha0+
Build ID: da7040510d5be27ad7b90ffb0c962535f3375358
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: threaded
Comment 18 Commit Notification 2018-08-20 09:59:40 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=81818670fe90fca438dfb0c22903474745112cfe&h=libreoffice-6-1

tdf#119224 start and end are expected to exist for the scope of this function

It will be available in 6.1.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 19 Commit Notification 2023-01-18 20:19:50 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/90e666baa16e167c733d0a0846ca6281daf1b099

sw: remove questionable assert that fires on tdf119224-1.docx

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.