diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2017-04-07 10:17:49 +0200 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2017-04-07 10:17:49 +0200 |
commit | d78040aadb6a8e3eeee1652b62a06d6e7e9b28ce (patch) | |
tree | 33ca205bfbee5189b91f6a04860613773b59929e /unoxml/source/dom/documentbuilder.cxx | |
parent | d01bd207da18fd47c23f408f0519140d0983ed7a (diff) |
ASan stack-use-after-scope
...as reported during CppunitTest_extensions_test_update (see below). I don't
see a reason why those pContext should be shared_ptr, so better make them
unique_ptr to guarantee that xmlFreeParserCtxt is called with c still in scope
(in CDocumentBuilder::parse).
> ==10883==ERROR: AddressSanitizer: stack-use-after-scope on address 0x2b111da94b68 at pc 0x2b116e55a200 bp 0x7fff7228b0f0 sp 0x7fff7228b0e8
> READ of size 8 at 0x2b111da94b68 thread T0
> #0 0x2b116e55a1ff in com::sun::star::uno::BaseReference::is() const include/com/sun/star/uno/Reference.h:94:27
> #1 0x2b116e699756 in DOM::xmlIO_close_func(void*) unoxml/source/dom/documentbuilder.cxx:214:33
> #2 0x2b11300cf646 in xmlFreeParserInputBuffer__internal_alias workdir/UnpackedTarball/xml2/xmlIO.c:2575:2
> #3 0x2b112fdd2706 in xmlFreeInputStream__internal_alias workdir/UnpackedTarball/xml2/parserInternals.c:1341:9
> #4 0x2b112fddebf3 in xmlFreeParserCtxt__internal_alias workdir/UnpackedTarball/xml2/parserInternals.c:1774:9
> #5 0x2b116e6aaddc in std::_Sp_counted_deleter<_xmlParserCtxt*, void (*)(_xmlParserCtxt*), std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr_base.h:464:9
> #6 0x2b116e65370a in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr_base.h:150:6
> #7 0x2b116e653494 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr_base.h:662:11
> #8 0x2b116e6a78b1 in std::__shared_ptr<_xmlParserCtxt, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr_base.h:928:31
> #9 0x2b116e69f01d in std::shared_ptr<_xmlParserCtxt>::~shared_ptr() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr.h:93:11
> #10 0x2b116e696aca in DOM::CDocumentBuilder::parse(com::sun::star::uno::Reference<com::sun::star::io::XInputStream> const&) unoxml/source/dom/documentbuilder.cxx:336:5
> #11 0x2b116e699fe2 in non-virtual thunk to DOM::CDocumentBuilder::parse(com::sun::star::uno::Reference<com::sun::star::io::XInputStream> const&) unoxml/source/dom/documentbuilder.cxx
> #12 0x2b116df6449a in (anonymous namespace)::UpdateInformationProvider::getUpdateInformationEnumeration(com::sun::star::uno::Sequence<rtl::OUString> const&, rtl::OUString const&) extensions/source/update/feed/updatefeed.cxx:589:83
> #13 0x2b116df682ea in non-virtual thunk to (anonymous namespace)::UpdateInformationProvider::getUpdateInformationEnumeration(com::sun::star::uno::Sequence<rtl::OUString> const&, rtl::OUString const&) extensions/source/update/feed/updatefeed.cxx
> #14 0x2b115d9a73eb in testupdate::Test::testGetUpdateInformationEnumeration() extensions/qa/update/test_update.cxx:57:26
> #15 0x2b115d9baf1b in CppUnit::TestCaller<testupdate::Test>::runTest() workdir/UnpackedTarball/cppunit/include/cppunit/TestCaller.h:166:6
> #16 0x2b111904ad8b in CppUnit::TestCaseMethodFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/TestCase.cpp:32:5
> #17 0x2b11322dab0f in (anonymous namespace)::Protector::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) test/source/vclbootstrapprotector.cxx:39:14
> #18 0x2b11190093ce in CppUnit::ProtectorChain::ProtectFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:20:25
> #19 0x2b1128c6214f in (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) unotest/source/cpp/unobootstrapprotector/unobootstrapprotector.cxx:89:12
> #20 0x2b11190093ce in CppUnit::ProtectorChain::ProtectFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:20:25
> #21 0x2b1124efc351 in (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) unotest/source/cpp/unoexceptionprotector/unoexceptionprotector.cxx:63:16
> #22 0x2b11190093ce in CppUnit::ProtectorChain::ProtectFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:20:25
> #23 0x2b1118f87350 in CppUnit::DefaultProtector::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) workdir/UnpackedTarball/cppunit/src/cppunit/DefaultProtector.cpp:15:12
> #24 0x2b11190093ce in CppUnit::ProtectorChain::ProtectFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:20:25
> #25 0x2b1119005e70 in CppUnit::ProtectorChain::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:77:18
> #26 0x2b11190c50f5 in CppUnit::TestResult::protect(CppUnit::Functor const&, CppUnit::Test*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) workdir/UnpackedTarball/cppunit/src/cppunit/TestResult.cpp:181:28
> #27 0x2b1119048fa4 in CppUnit::TestCase::run(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestCase.cpp:91:13
> #28 0x2b111904d7a7 in CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestComposite.cpp:64:30
> #29 0x2b111904c819 in CppUnit::TestComposite::run(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestComposite.cpp:23:3
> #30 0x2b111904d7a7 in CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestComposite.cpp:64:30
> #31 0x2b111904c819 in CppUnit::TestComposite::run(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestComposite.cpp:23:3
> #32 0x2b11191035c9 in CppUnit::TestRunner::WrappingSuite::run(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestRunner.cpp:47:27
> #33 0x2b11190c340d in CppUnit::TestResult::runTest(CppUnit::Test*) workdir/UnpackedTarball/cppunit/src/cppunit/TestResult.cpp:148:9
> #34 0x2b111910489b in CppUnit::TestRunner::run(CppUnit::TestResult&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) workdir/UnpackedTarball/cppunit/src/cppunit/TestRunner.cpp:96:14
> #35 0x532fb4 in (anonymous namespace)::ProtectedFixtureFunctor::run() const sal/cppunittester/cppunittester.cxx:306:20
> #36 0x52e7c3 in sal_main() sal/cppunittester/cppunittester.cxx:456:20
> #37 0x52cb6f in main sal/cppunittester/cppunittester.cxx:363:1
> #38 0x2b111acb5400 in __libc_start_main (/lib64/libc.so.6+0x20400)
> #39 0x438019 in _start (workdir/LinkTarget/Executable/cppunittester+0x438019)
>
> Address 0x2b111da94b68 is located in stack of thread T0 at offset 104 in frame
> #0 0x2b116e69553f in DOM::CDocumentBuilder::parse(com::sun::star::uno::Reference<com::sun::star::io::XInputStream> const&) unoxml/source/dom/documentbuilder.cxx:303
>
> This frame has 4 object(s):
> [32, 40) 'g' (line 308)
> [64, 80) 'pContext' (line 310)
> [96, 120) 'c' (line 321) <== Memory access at offset 104 is inside this variable
> [160, 168) 'ref.tmp' (line 333)
Change-Id: I3398d54776af4927417e392e55bbca740d4121af
Diffstat (limited to 'unoxml/source/dom/documentbuilder.cxx')
-rw-r--r-- | unoxml/source/dom/documentbuilder.cxx | 32 |
1 files changed, 21 insertions, 11 deletions
diff --git a/unoxml/source/dom/documentbuilder.cxx b/unoxml/source/dom/documentbuilder.cxx index b8f79f6fe9a8..71a6dc6f1785 100644 --- a/unoxml/source/dom/documentbuilder.cxx +++ b/unoxml/source/dom/documentbuilder.cxx @@ -299,6 +299,14 @@ namespace DOM throw saxex; } + namespace { + + struct XmlFreeParserCtxt { + void operator ()(xmlParserCtxt * p) const { xmlFreeParserCtxt(p); } + }; + + } + Reference< XDocument > SAL_CALL CDocumentBuilder::parse(const Reference< XInputStream >& is) { if (!is.is()) { @@ -307,8 +315,17 @@ namespace DOM ::osl::MutexGuard const g(m_Mutex); - std::shared_ptr<xmlParserCtxt> const pContext( - xmlNewParserCtxt(), xmlFreeParserCtxt); + // IO context struct. Must outlive pContext, as destroying that via + // xmlFreeParserCtxt may still access this context_t + context_t c; + c.pBuilder = this; + c.rInputStream = is; + // we did not open the stream, thus we do not close it. + c.close = false; + c.freeOnClose = false; + + std::unique_ptr<xmlParserCtxt, XmlFreeParserCtxt> const pContext( + xmlNewParserCtxt()); // register error functions to prevent errors being printed // on the console @@ -317,13 +334,6 @@ namespace DOM pContext->sax->warning = warning_func; pContext->sax->resolveEntity = resolve_func; - // IO context struct - context_t c; - c.pBuilder = this; - c.rInputStream = is; - // we did not open the stream, thus we do not close it. - c.close = false; - c.freeOnClose = false; xmlDocPtr const pDoc = xmlCtxtReadIO(pContext.get(), xmlIO_read_func, xmlIO_close_func, &c, nullptr, nullptr, 0); @@ -339,8 +349,8 @@ namespace DOM { ::osl::MutexGuard const g(m_Mutex); - std::shared_ptr<xmlParserCtxt> const pContext( - xmlNewParserCtxt(), xmlFreeParserCtxt); + std::unique_ptr<xmlParserCtxt, XmlFreeParserCtxt> const pContext( + xmlNewParserCtxt()); pContext->_private = this; pContext->sax->error = error_func; pContext->sax->warning = warning_func; |