• Stephan Bergmann's avatar
    Fix use-after-free in SwLayAction::IsShortCut · 554e0905
    Stephan Bergmann yazdı
    ...as seen with valgrind during CppunitTest_sw_odfexport (see below).
    
    It looks like the
    
      if ( !IsEmptyPage() ) //#59184# unnecessary for empty pages
    
    optimization in SwPageFrame::DestroyImpl (which was there ever since at least
    84a3db80 "initial import") was wrong, preventing
    the call to
    
      pImp->GetLayAction().SetAgain();
    
    that would cause SwLayAction::IsShortCut (sw/source/core/layout/layact.cxx) to
    quit early from an IsAgain() call, before accessing prPage again that has
    meanwhile been destroyed from within its
    
      pContent->Calc(pRenderContext);
    
    call.
    
    The same failure started to show in ASan+UBSan builds only now after
    integration of <https://gerrit.libreoffice.org/#/c/58263/> "the custom SAL
    allocator is no longer used", for reasons I explained in a comment there:  "For
    FORCE_SYSALLOC (which was esp. set for ASan/UBSan builds), alloc_mode was always
    left at AllocMode::UNSET (as determine_alloc_mode was never called in
    FORCE_SYSALLOC blocks), so rtl_cache_alloc (which only checked alloc_mode for
    AllocMode::SYSTEM, but never called determine_alloc, and wasn't entirely short-
    circuited under FORCE_SYSALLOC) actually called into the legacy, supposed-dead
    slab allocator code.  That's apparently how some use-after-free bug in sw got
    hidden from ASan+UBSan builds in the past, and only now starts to show up
    (<https://ci.libreoffice.org/job/lo_ubsan/989/>)."
    
    The valgrind failure log:
    [...]
    > testFdo58949::Import_Export_Import finished in: 191975ms
    > File tested,Execution Time (ms)
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > warn:sw:12555:12555:sw/inc/swrect.hxx:283: SVRect() without Width or Height
    > ==12555== Invalid read of size 8
    > ==12555==    at 0x2AFDAA7C: SwFrame::GetPrev() (/sw/source/core/inc/frame.hxx:649)
    > ==12555==    by 0x2B6A4F7A: SwLayAction::IsShortCut(SwPageFrame*&) (/sw/source/core/layout/layact.cxx:1071)
    > ==12555==    by 0x2B6A342D: SwLayAction::InternalAction(OutputDevice*) (/sw/source/core/layout/layact.cxx:473)
    > ==12555==    by 0x2B6A2E46: SwLayAction::Action(OutputDevice*) (/sw/source/core/layout/layact.cxx:340)
    > ==12555==    by 0x2BE0F8A2: SwViewShell::ImplEndAction(bool) (/sw/source/core/view/viewsh.cxx:281)
    > ==12555==    by 0x2AF7A6C0: SwViewShell::EndAction(bool) (/sw/inc/viewsh.hxx:595)
    > ==12555==    by 0x2AF68B50: SwCursorShell::EndAction(bool, bool) (/sw/source/core/crsr/crsrsh.cxx:258)
    > ==12555==    by 0x2C464281: SwView::OuterResizePixel(Point const&, Size const&) (/sw/source/uibase/uiview/viewport.cxx:1124)
    > ==12555==    by 0x2A29D78F: SfxViewFrame::DoAdjustPosSizePixel(SfxViewShell*, Point const&, Size const&, bool) (/sfx2/source/view/viewfrm.cxx:1604)
    > ==12555==    by 0x2A2A390E: SfxViewFrame::Resize(bool) (/sfx2/source/view/viewfrm.cxx:2395)
    > ==12555==    by 0x2A2AF1A7: SfxFrameViewWindow_Impl::Resize() (/sfx2/source/view/viewfrm2.cxx:73)
    > ==12555==    by 0x1A8C2A64: vcl::Window::ImplCallResize() (/vcl/source/window/event.cxx:523)
    > ==12555==    by 0x1AA2AF8D: vcl::Window::Show(bool, ShowFlags) (/vcl/source/window/window.cxx:2277)
    > ==12555==    by 0x2A282069: SfxBaseController::ConnectSfxFrame_Impl(SfxBaseController::ConnectSfxFrame) (/sfx2/source/view/sfxbasecontroller.cxx:1250)
    > ==12555==    by 0x2A2815BA: SfxBaseController::attachFrame(com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&) (/sfx2/source/view/sfxbasecontroller.cxx:550)
    > ==12555==    by 0x2A265C78: (anonymous namespace)::SfxFrameLoader_Impl::impl_createDocumentView(com::sun::star::uno::Reference<com::sun::star::frame::XModel2> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, comphelper::NamedValueCollection const&, rtl::OUString const&) (/sfx2/source/view/frmload.cxx:594)
    > ==12555==    by 0x2A26374F: (anonymous namespace)::SfxFrameLoader_Impl::load(com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&) (/sfx2/source/view/frmload.cxx:711)
    > ==12555==    by 0x36E8A2A1: framework::LoadEnv::impl_loadContent() (/framework/source/loadenv/loadenv.cxx:1149)
    > ==12555==    by 0x36E84F2A: framework::LoadEnv::startLoading() (/framework/source/loadenv/loadenv.cxx:383)
    > ==12555==    by 0x36E83979: framework::LoadEnv::loadComponentFromURL(com::sun::star::uno::Reference<com::sun::star::frame::XComponentLoader> const&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) (/framework/source/loadenv/loadenv.cxx:169)
    > ==12555==    by 0x36EDDEB8: framework::Desktop::loadComponentFromURL(rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) (/framework/source/services/desktop.cxx:619)
    > ==12555==    by 0x36EDDF6A: non-virtual thunk to framework::Desktop::loadComponentFromURL(rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) (/framework/source/services/desktop.cxx:0)
    > ==12555==    by 0x2D456C63: unotest::MacrosTest::loadFromDesktop(rtl::OUString const&, rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) (/unotest/source/cpp/macros_test.cxx:50)
    > ==12555==    by 0x295BCC30: SwModelTestBase::loadURL(rtl::OUString const&, char const*, char const*) (/sw/qa/extras/inc/swmodeltestbase.hxx:762)
    > ==12555==    by 0x295BC7E9: SwModelTestBase::load(rtl::OUString const&, char const*, char const*) (/sw/qa/extras/inc/swmodeltestbase.hxx:717)
    > ==12555==    by 0x295BC660: SwModelTestBase::executeImportTest(char const*, char const*) (/sw/qa/extras/inc/swmodeltestbase.hxx:264)
    > ==12555==    by 0x295E4B5F: testStylePageNumber::Import() (/sw/qa/extras/odfexport/odfexport.cxx:686)
    [...]
    > ==12555==  Address 0x28ddde10 is 160 bytes inside a block of size 280 free'd
    > ==12555==    at 0x4C2FDAC: free (/builddir/build/BUILD/valgrind-3.13.0/coregrind/m_replacemalloc/vg_replace_malloc.c:530)
    > ==12555==    by 0x519E494: rtl_freeMemory (/sal/rtl/alloc_global.cxx:51)
    > ==12555==    by 0x519DCDB: rtl_cache_free (/sal/rtl/alloc_cache.cxx:172)
    > ==12555==    by 0x19BB090A: FixedMemPool::Free(void*) (/tools/source/memtools/mempool.cxx:49)
    > ==12555==    by 0x2B68B89D: SwPageFrame::operator delete(void*, unsigned long) (/sw/source/core/inc/pagefrm.hxx:111)
    > ==12555==    by 0x2B6DA2E8: SwPageFrame::~SwPageFrame() (/sw/source/core/layout/pagechg.cxx:301)
    > ==12555==    by 0x2B74E763: SwFrame::DestroyFrame(SwFrame*) (/sw/source/core/layout/ssfrm.cxx:384)
    > ==12555==    by 0x2B6E18D3: SwRootFrame::RemovePage(SwPageFrame**, SwRemoveResult) (/sw/source/core/layout/pagechg.cxx:1414)
    > ==12555==    by 0x2B6E145D: (anonymous namespace)::doInsertPage(SwRootFrame*, SwPageFrame**, SwFrameFormat*, SwPageDesc*, bool, SwPageFrame**) (/sw/source/core/layout/pagechg.cxx:1270)
    > ==12555==    by 0x2B6E0CA0: SwFrame::InsertPage(SwPageFrame*, bool) (/sw/source/core/layout/pagechg.cxx:1324)
    > ==12555==    by 0x2B65373D: SwFrame::GetNextLeaf(MakePageType) (/sw/source/core/layout/flowfrm.cxx:997)
    > ==12555==    by 0x2B65331A: SwFrame::GetLeaf(MakePageType, bool) (/sw/source/core/layout/flowfrm.cxx:818)
    > ==12555==    by 0x2B657637: SwFlowFrame::MoveFwd(bool, bool, bool) (/sw/source/core/layout/flowfrm.cxx:1876)
    > ==12555==    by 0x2B657133: SwFlowFrame::CheckMoveFwd(bool&, bool, bool) (/sw/source/core/layout/flowfrm.cxx:1796)
    > ==12555==    by 0x2B634109: SwContentFrame::MakeAll(OutputDevice*) (/sw/source/core/layout/calcmove.cxx:1322)
    > ==12555==    by 0x2B62CEF4: SwFrame::PrepareMake(OutputDevice*) (/sw/source/core/layout/calcmove.cxx:343)
    > ==12555==    by 0x2B770166: SwFrame::Calc(OutputDevice*) const (/sw/source/core/layout/trvlfrm.cxx:1799)
    > ==12555==    by 0x2B6A4F11: SwLayAction::IsShortCut(SwPageFrame*&) (/sw/source/core/layout/layact.cxx:1065)
    > ==12555==    by 0x2B6A342D: SwLayAction::InternalAction(OutputDevice*) (/sw/source/core/layout/layact.cxx:473)
    > ==12555==    by 0x2B6A2E46: SwLayAction::Action(OutputDevice*) (/sw/source/core/layout/layact.cxx:340)
    > ==12555==    by 0x2BE0F8A2: SwViewShell::ImplEndAction(bool) (/sw/source/core/view/viewsh.cxx:281)
    [...]
    
    Change-Id: I57ebbab536ca41554e4681477cf1dea62abbc688
    Reviewed-on: https://gerrit.libreoffice.org/58550
    Tested-by: Jenkins
    Reviewed-by: 's avatarStephan Bergmann <sbergman@redhat.com>
    554e0905
Adı
Son kayıt (commit)
Son güncelleme
.git-hooks Loading commit data...
UnoControls Loading commit data...
accessibility Loading commit data...
android Loading commit data...
animations Loading commit data...
apple_remote Loading commit data...
avmedia Loading commit data...
basctl Loading commit data...
basegfx Loading commit data...
basic Loading commit data...
bean Loading commit data...
bin Loading commit data...
binaryurp Loading commit data...
bridges Loading commit data...
canvas Loading commit data...
chart2 Loading commit data...
cli_ure Loading commit data...
codemaker Loading commit data...
comphelper Loading commit data...
compilerplugins Loading commit data...
config_host Loading commit data...
configmgr Loading commit data...
connectivity Loading commit data...
cppcanvas Loading commit data...
cppu Loading commit data...
cppuhelper Loading commit data...
cpputools Loading commit data...
cui Loading commit data...
dbaccess Loading commit data...
desktop Loading commit data...
dictionaries @ 00a2316c
distro-configs Loading commit data...
drawinglayer Loading commit data...
dtrans Loading commit data...
editeng Loading commit data...
embeddedobj Loading commit data...
embedserv Loading commit data...
emfio Loading commit data...
eventattacher Loading commit data...
extensions Loading commit data...
external Loading commit data...
extras Loading commit data...
filter Loading commit data...
forms Loading commit data...
formula Loading commit data...
fpicker Loading commit data...
framework Loading commit data...
helpcompiler Loading commit data...
helpcontent2 @ cfc1d042
hwpfilter Loading commit data...
i18nlangtag Loading commit data...
i18npool Loading commit data...
i18nutil Loading commit data...
icon-themes Loading commit data...
idl Loading commit data...
idlc Loading commit data...
include Loading commit data...
instsetoo_native Loading commit data...
io Loading commit data...
ios Loading commit data...
javaunohelper Loading commit data...
jurt Loading commit data...
jvmaccess Loading commit data...
jvmfwk Loading commit data...
l10ntools Loading commit data...
librelogo Loading commit data...
libreofficekit Loading commit data...
lingucomponent Loading commit data...
linguistic Loading commit data...
lotuswordpro Loading commit data...
m4 Loading commit data...
mysqlc Loading commit data...
nlpsolver Loading commit data...
o3tl Loading commit data...
odk Loading commit data...
offapi Loading commit data...
officecfg Loading commit data...
onlineupdate Loading commit data...
oovbaapi Loading commit data...
oox Loading commit data...
opencl Loading commit data...
osx Loading commit data...
package Loading commit data...
postprocess Loading commit data...
pyuno Loading commit data...
qadevOOo Loading commit data...
readlicense_oo Loading commit data...
registry Loading commit data...
remotebridges Loading commit data...
reportbuilder Loading commit data...
reportdesign Loading commit data...
ridljar Loading commit data...
sal Loading commit data...
salhelper Loading commit data...
sax Loading commit data...
sc Loading commit data...
scaddins Loading commit data...
sccomp Loading commit data...
schema Loading commit data...
scp2 Loading commit data...
scripting Loading commit data...
sd Loading commit data...
sdext Loading commit data...
setup_native Loading commit data...
sfx2 Loading commit data...
shell Loading commit data...
slideshow Loading commit data...
smoketest Loading commit data...
solenv Loading commit data...
soltools Loading commit data...
sot Loading commit data...
starmath Loading commit data...
stoc Loading commit data...
store Loading commit data...
svgio Loading commit data...
svl Loading commit data...
svtools Loading commit data...
svx Loading commit data...
sw Loading commit data...
swext Loading commit data...
sysui Loading commit data...
test Loading commit data...
testtools Loading commit data...
toolkit Loading commit data...
tools Loading commit data...
translations @ 54cc0de6
ucb Loading commit data...
ucbhelper Loading commit data...
udkapi Loading commit data...
uitest Loading commit data...
unodevtools Loading commit data...
unoidl Loading commit data...
unoil Loading commit data...
unotest Loading commit data...
unotools Loading commit data...
unoxml Loading commit data...
ure Loading commit data...
uui Loading commit data...
vbahelper Loading commit data...
vcl Loading commit data...
winaccessibility Loading commit data...
wizards Loading commit data...
writerfilter Loading commit data...
writerperfect Loading commit data...
xmerge Loading commit data...
xmlhelp Loading commit data...
xmloff Loading commit data...
xmlreader Loading commit data...
xmlscript Loading commit data...
xmlsecurity Loading commit data...
.buckconfig Loading commit data...
.buckversion Loading commit data...
.clang-format Loading commit data...
.editorconfig Loading commit data...
.gitattributes Loading commit data...
.gitignore Loading commit data...
.gitmodules Loading commit data...
.gitreview Loading commit data...
BUCK Loading commit data...
COPYING Loading commit data...
COPYING.LGPL Loading commit data...
COPYING.MPL Loading commit data...
Library_merged.mk Loading commit data...
Makefile.fetch Loading commit data...
Makefile.gbuild Loading commit data...
Makefile.in Loading commit data...
README.Solaris Loading commit data...
README.cross Loading commit data...
README.md Loading commit data...
Repository.mk Loading commit data...
RepositoryExternal.mk Loading commit data...
RepositoryFixes.mk Loading commit data...
RepositoryModule_build.mk Loading commit data...
RepositoryModule_host.mk Loading commit data...
TEMPLATE.SOURCECODE.HEADER Loading commit data...
autogen.sh Loading commit data...
config.guess Loading commit data...
config.sub Loading commit data...
config_host.mk.in Loading commit data...
config_host_lang.mk.in Loading commit data...
configure.ac Loading commit data...
download.lst Loading commit data...
g Loading commit data...
install-sh Loading commit data...
leak-suppress.txt Loading commit data...
lo.xcent.in Loading commit data...
logerrit Loading commit data...
sanitize-ubsan-blacklist Loading commit data...