tdf#64711 pointer deleted out from under std::shared_ptr

the "end" call will close the shell, which is deleted directly so the local shared_ptr remains
pointed to something which is already deleted.

So:
a) Have activate return true/false to indicate the failure and require the caller to call
'end' in response to failure.
b) A bunch of stuff in the call-stack expects the ViewShell not to get deleted while they are
calling it, so additionally launch that 'end' to happen in the next event loop.

==2838867== Invalid read of size 8
==2838867==    at 0x32F4B83C: sd::ViewShellBase::GetDocShell() const (ViewShellBase.hxx:97)
==2838867==    by 0x335BBCFC: sd::ViewShell::GetDocSh() const (viewshel.cxx:1389)
==2838867==    by 0x3357CAC1: sd::PresentationViewShell::~PresentationViewShell() (presvish.cxx:73)
==2838867==    by 0x330AA34B: void __gnu_cxx::new_allocator<sd::PresentationViewShell>::destroy<sd::PresentationViewShell>(sd::PresentationViewShell*) (new_allocator.h:156)
==2838867==    by 0x330AA2DF: void std::allocator_traits<std::allocator<sd::PresentationViewShell> >::destroy<sd::PresentationViewShell>(std::allocator<sd::PresentationViewShell>&, sd::PresentationViewShell*) (alloc_traits.h:531)
==2838867==    by 0x330AA0BE: std::_Sp_counted_ptr_inplace<sd::PresentationViewShell, std::allocator<sd::PresentationViewShell>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:560)
==2838867==    by 0x32D10D33: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:158)
==2838867==    by 0x32D10C79: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:733)
==2838867==    by 0x330A03BD: std::__shared_ptr<sd::PresentationViewShell, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1183)
==2838867==    by 0x3309F847: std::shared_ptr<sd::PresentationViewShell>::~shared_ptr() (shared_ptr.h:121)
==2838867==    by 0x3320E1E8: sd::SlideShow::activate(sd::ViewShellBase&) (slideshow.cxx:999)
==2838867==    by 0x3357CF33: sd::PresentationViewShell::Activate(bool) (presvish.cxx:118)

==2838867==  Address 0x2fb5f320 is 256 bytes inside a block of size 272 free'd
==2838867==    at 0x483BEDD: operator delete(void*) (vg_replace_malloc.c:584)
==2838867==    by 0x33466537: sd::PresentationViewShellBase::~PresentationViewShellBase() (PresentationViewShellBase.cxx:82)
==2838867==    by 0x823076C: SfxViewFrame::ReleaseObjectShell_Impl() (viewfrm.cxx:1113)
==2838867==    by 0x8234B1C: SfxViewFrame::~SfxViewFrame() (viewfrm.cxx:1652)
==2838867==    by 0x8234FEB: SfxViewFrame::~SfxViewFrame() (viewfrm.cxx:1646)
==2838867==    by 0x8230D6C: SfxViewFrame::Close() (viewfrm.cxx:1165)
==2838867==    by 0x81E4217: SfxFrame::DoClose_Impl() (frame.cxx:142)
==2838867==    by 0x821709A: SfxBaseController::dispose() (sfxbasecontroller.cxx:982)
==2838867==    by 0x3337A59F: sd::DrawController::dispose() (DrawController.cxx:164)
==2838867==    by 0x6F35CC0: (anonymous namespace)::XFrameImpl::setComponent(com::sun::uno::Reference<com::sun::awt::XWindow> const&, com::sun::uno::Reference<com::sun::frame::XController> const&) (frame.cxx:1485)
==2838867==    by 0x6F3834E: (anonymous namespace)::XFrameImpl::close(unsigned char) (frame.cxx:1692)
==2838867==    by 0x81E3CFA: SfxFrame::DoClose() (frame.cxx:108)
==2838867==    by 0x823802C: SfxViewFrame::DoClose() (viewfrm.cxx:2525)
==2838867==    by 0x3320B971: sd::SlideShow::end() (slideshow.cxx:696)
==2838867==    by 0x3320E1C2: sd::SlideShow::activate(sd::ViewShellBase&) (slideshow.cxx:995)
==2838867==    by 0x3357CF33: sd::PresentationViewShell::Activate(bool) (presvish.cxx:118)

Change-Id: Ida91228b7584491c9a5dc9c0b3f76ce63218a92a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103319
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
This commit is contained in:
Caolán McNamara
2020-09-24 14:00:35 +01:00
parent b5e023a7ef
commit 582a73abd2
5 changed files with 45 additions and 17 deletions

View File

@@ -58,9 +58,12 @@ protected:
private:
::tools::Rectangle maOldVisArea;
ImplSVEvent* mnAbortSlideShowEvent;
virtual void Activate (bool bIsMDIActivate) override;
virtual void Paint (const ::tools::Rectangle& rRect, ::sd::Window* pWin) override;
DECL_LINK(AbortSlideShowHdl, void*, void);
};
} // end of namespace sd

View File

@@ -157,7 +157,8 @@ public:
// events
void resize( const Size &rSize );
void activate(ViewShellBase& rBase);
// return false if the activate failed. callers should call end in response to failre
bool activate(ViewShellBase& rBase);
void deactivate();
void paint();

View File

@@ -972,7 +972,7 @@ void SlideShow::resize( const Size &rSize )
mxController->resize( rSize );
}
void SlideShow::activate( ViewShellBase& rBase )
bool SlideShow::activate( ViewShellBase& rBase )
{
if( (mpFullScreenViewShellBase == &rBase) && !mxController.is() )
{
@@ -984,23 +984,19 @@ void SlideShow::activate( ViewShellBase& rBase )
CreateController( pShell.get(), pShell->GetView(), rBase.GetViewWindow() );
if( mxController->startShow(mxCurrentSettings.get()) )
{
pShell->Resize();
// Defer the sd::ShowWindow's GrabFocus to here. so that the accessible event can be fired correctly.
pShell->GetActiveWindow()->GrabFocus();
}
else
{
end();
return;
}
if (!mxController->startShow(mxCurrentSettings.get()))
return false;
pShell->Resize();
// Defer the sd::ShowWindow's GrabFocus to here. so that the accessible event can be fired correctly.
pShell->GetActiveWindow()->GrabFocus();
}
}
if( mxController.is() )
mxController->activate();
return true;
}
void SlideShow::deactivate()

View File

@@ -61,7 +61,8 @@ void PresentationViewShell::InitInterface_Impl()
PresentationViewShell::PresentationViewShell( ViewShellBase& rViewShellBase, vcl::Window* pParentWindow, FrameView* pFrameView)
: DrawViewShell( rViewShellBase, pParentWindow, PageKind::Standard, pFrameView)
: DrawViewShell(rViewShellBase, pParentWindow, PageKind::Standard, pFrameView)
, mnAbortSlideShowEvent(nullptr)
{
if( GetDocSh() && GetDocSh()->GetCreateMode() == SfxObjectCreateMode::EMBEDDED )
maOldVisArea = GetDocSh()->GetVisArea( ASPECT_CONTENT );
@@ -70,6 +71,9 @@ PresentationViewShell::PresentationViewShell( ViewShellBase& rViewShellBase, vcl
PresentationViewShell::~PresentationViewShell()
{
if (mnAbortSlideShowEvent)
Application::RemoveUserEvent(mnAbortSlideShowEvent);
if( GetDocSh() && GetDocSh()->GetCreateMode() == SfxObjectCreateMode::EMBEDDED && !maOldVisArea.IsEmpty() )
GetDocSh()->SetVisArea( maOldVisArea );
}
@@ -102,6 +106,14 @@ VclPtr<SvxRuler> PresentationViewShell::CreateVRuler(::sd::Window*)
return nullptr;
}
IMPL_LINK_NOARG(PresentationViewShell, AbortSlideShowHdl, void*, void)
{
mnAbortSlideShowEvent = nullptr;
rtl::Reference<SlideShow> xSlideShow(SlideShow::GetSlideShow(GetViewShellBase()));
if (xSlideShow.is())
xSlideShow->end();
}
void PresentationViewShell::Activate( bool bIsMDIActivate )
{
DrawViewShell::Activate( bIsMDIActivate );
@@ -115,7 +127,20 @@ void PresentationViewShell::Activate( bool bIsMDIActivate )
rtl::Reference< SlideShow > xSlideShow( SlideShow::GetSlideShow( GetViewShellBase() ) );
if( xSlideShow.is() )
xSlideShow->activate(GetViewShellBase());
{
bool bSuccess = xSlideShow->activate(GetViewShellBase());
if (!bSuccess)
{
/* tdf#64711 PresentationViewShell is deleted by 'end' due to end closing
the object shell. So if we call xSlideShow->end during Activate there are
a lot of places in the call stack of Activate which understandable don't
expect this ViewShell to be deleted during use. Defer to the next event
loop the abort of the slideshow
*/
if (!mnAbortSlideShowEvent)
mnAbortSlideShowEvent = Application::PostUserEvent(LINK(this, PresentationViewShell, AbortSlideShowHdl));
}
}
if( HasCurrentFunction() )
GetCurrentFunction()->Activate();

View File

@@ -320,8 +320,11 @@ void ViewShell::Activate(bool bIsMDIActivate)
rBindings.Invalidate( SID_3D_STATE, true );
rtl::Reference< SlideShow > xSlideShow( SlideShow::GetSlideShow( GetViewShellBase() ) );
if(xSlideShow.is() && xSlideShow->isRunning() )
xSlideShow->activate(GetViewShellBase());
if (xSlideShow.is() && xSlideShow->isRunning())
{
bool bSuccess = xSlideShow->activate(GetViewShellBase());
assert(bSuccess && "can only return false with a PresentationViewShell"); (void)bSuccess;
}
if(HasCurrentFunction())
GetCurrentFunction()->Activate();