| Message ID | CAPNORr-c-oe5Bw_d-ZE9k4TcD6pnS7QQuz0Usej_oFdUKyRoPw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 03/04/2016 01:48 PM, Ivan Koldaev wrote: > Patch for uClibc++ bug #8741 (https://bugs.busybox.net/show_bug.cgi?id=8741) > > I have discovered that uClibc++ incorrectly implements C++ exception > ABI, allocating not enough memory in the > "__cxxabiv1::__cxa_allocate_exception": When I spoke to Garrett Kajmowicz on the phone last month, he said that his previous day job stuck with last GPLv2 release of gcc, so he stopped staying up to date with changes in the C++ specification after ~2007. Meaning he hasn't touched uClibc++ in years. He switched jobs recently (at Google now) but didn't seem particularly interested in picking up where he left off after a multi-year gap. (I think they use LLVM internally there, and thus http://libcxx.llvm.org/ ? I know Android and ChromeOS do, dunno about the web server plumbing stuff he's dealing with. It sounded more like AI research than anything else, really...) I still use it in Aboriginal Linux, but only because I haven't switched my toolchain to LLVM yet... Rob
On Fri, Mar 11, 2016 at 1:41 PM, Rob Landley <rob@landley.net> wrote: > On 03/04/2016 01:48 PM, Ivan Koldaev wrote: >> Patch for uClibc++ bug #8741 (https://bugs.busybox.net/show_bug.cgi?id=8741) >> >> I have discovered that uClibc++ incorrectly implements C++ exception >> ABI, allocating not enough memory in the >> "__cxxabiv1::__cxa_allocate_exception": > > When I spoke to Garrett Kajmowicz on the phone last month, he said that > his previous day job stuck with last GPLv2 release of gcc, so he stopped > staying up to date with changes in the C++ specification after ~2007. > Meaning he hasn't touched uClibc++ in years. > > He switched jobs recently (at Google now) but didn't seem particularly > interested in picking up where he left off after a multi-year gap. (I > think they use LLVM internally there, and thus http://libcxx.llvm.org/ ? > I know Android and ChromeOS do, dunno about the web server plumbing > stuff he's dealing with. It sounded more like AI research than anything > else, really...) > > I still use it in Aboriginal Linux, but only because I haven't switched > my toolchain to LLVM yet... > > Rob Rob, I appreciate your input. I understand that original author stepped away from the uClibc++ development, but as I understand uClibc++ is not completely abandoned project, since there are still commits dated 6 days ago (https://git.uclibc.org/uClibc++/log/) by Bernhard Reutner-Fischer. Also OpenWRT uses uClibc++ as a primary C++ library with the latest GCC 5.2.x (luckily almost no program is using exceptions on embedded). So if you are saying that posting to this list is not a right way to have my patch accepted, could you please point me to the right one. Thank you for your time. P.S. added Bernhard Reutner-Fischer into CC, because I hope for some movement for this patch.
On March 12, 2016 12:18:02 AM GMT+01:00, Ivan Koldaev <pixus.ru@gmail.com> wrote: >On Fri, Mar 11, 2016 at 1:41 PM, Rob Landley <rob@landley.net> wrote: >> On 03/04/2016 01:48 PM, Ivan Koldaev wrote: >>> Patch for uClibc++ bug #8741 >(https://bugs.busybox.net/show_bug.cgi?id=8741) >>> >>> I have discovered that uClibc++ incorrectly implements C++ exception >>> ABI, allocating not enough memory in the >>> "__cxxabiv1::__cxa_allocate_exception": >> >> When I spoke to Garrett Kajmowicz on the phone last month, he said >that >> his previous day job stuck with last GPLv2 release of gcc, so he >stopped >> staying up to date with changes in the C++ specification after ~2007. >> Meaning he hasn't touched uClibc++ in years. >> >> He switched jobs recently (at Google now) but didn't seem >particularly >> interested in picking up where he left off after a multi-year gap. (I >> think they use LLVM internally there, and thus >http://libcxx.llvm.org/ ? >> I know Android and ChromeOS do, dunno about the web server plumbing >> stuff he's dealing with. It sounded more like AI research than >anything >> else, really...) >> >> I still use it in Aboriginal Linux, but only because I haven't >switched >> my toolchain to LLVM yet... >> >> Rob > >Rob, > >I appreciate your input. >I understand that original author stepped away from the uClibc++ >development, >but as I understand uClibc++ is not completely abandoned project, >since there are still commits dated 6 days ago >(https://git.uclibc.org/uClibc++/log/) by Bernhard Reutner-Fischer. >Also OpenWRT uses uClibc++ as a primary C++ library with the latest >GCC 5.2.x (luckily almost no program is using exceptions on embedded). >So if you are saying that posting to this list is not a right way to >have my patch accepted, could you please point me to the right one. >Thank you for your time. > >P.S. added Bernhard Reutner-Fischer into CC, because I hope for some >movement for this patch. I have this scheduled, thanks. Will push together with a second patch as soon as I have written a trivial testcase for the second buglet. Thanks for the patch! Cheers,
On 03/12/2016 11:20 AM, Bernhard Reutner-Fischer wrote: > On March 12, 2016 12:18:02 AM GMT+01:00, Ivan Koldaev <pixus.ru@gmail.com> wrote: >> P.S. added Bernhard Reutner-Fischer into CC, because I hope for some >> movement for this patch. > > I have this scheduled, thanks. Will push together with a second patch as soon as I have written a trivial testcase for the second buglet. > > Thanks for the patch! > Cheers, Ah, I thought the project was abandoned just because there hasn't been a release in years. My bad, I should know better around here. Rob
diff --git a/include/unwind-cxx.h b/include/unwind-cxx.h index b773259..03e6428 100644 --- a/include/unwind-cxx.h +++ b/include/unwind-cxx.h @@ -1,5 +1,5 @@ // -*- C++ -*- Exception handling and frame unwind runtime interface routines. -// Copyright (C) 2001 Free Software Foundation, Inc. +// Copyright (C) 2001-2015 Free Software Foundation, Inc. // // This file is part of GCC. // @@ -13,6 +13,10 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. // +// Under Section 7 of GPL version 3, you are granted additional +// permissions described in the GCC Runtime Library Exception, version +// 3.1, as published by the Free Software Foundation. +// // You should have received a copy of the GNU General Public License // along with GCC; see the file COPYING. If not, write to // the Free Software Foundation, 59 Temple Place - Suite 330, @@ -40,6 +44,12 @@ #include <cstddef> #include "unwind.h" +// Original unwind-cxx.h also includes bits/atomic_word.h which is CPU-specific, +// but always defines _Atomic_word as typedef int . +// Only thing that differs is memory-barrier macroses. +typedef int _Atomic_word; + + #pragma GCC visibility push(default) namespace __cxxabiv1 @@ -79,6 +89,13 @@ struct __cxa_exception _Unwind_Exception unwindHeader; }; +struct __cxa_refcounted_exception +{ + // Manage this header. + _Atomic_word referenceCount; + // __cxa_exception must be last, and no padding can be after it. + __cxa_exception exc; +}; // A dependent C++ exception object consists of a header, which is a wrapper // around an unwind object header with additional C++ specific information, @@ -210,6 +227,21 @@ __get_exception_header_from_ue (_Unwind_Exception *exc) return reinterpret_cast<__cxa_exception *>(exc + 1) - 1; } +// Acquire the C++ refcounted exception header from the C++ object. +static inline __cxa_refcounted_exception * +__get_refcounted_exception_header_from_obj (void *ptr) +{ + return reinterpret_cast<__cxa_refcounted_exception *>(ptr) - 1; +} + +// Acquire the C++ refcounted exception header from the generic exception +// header. +static inline __cxa_refcounted_exception * +__get_refcounted_exception_header_from_ue (_Unwind_Exception *exc) +{ + return reinterpret_cast<__cxa_refcounted_exception *>(exc + 1) - 1; +} + } /* namespace __cxxabiv1 */ #pragma GCC visibility pop diff --git a/src/eh_alloc.cpp b/src/eh_alloc.cpp index 5098196..5feeae9 100644 --- a/src/eh_alloc.cpp +++ b/src/eh_alloc.cpp @@ -30,16 +30,16 @@ extern "C" void * __cxa_allocate_exception(std::size_t thrown_size) throw(){ void *retval; //The sizeof crap is required by Itanium ABI because we need to provide space for //accounting information which is implementaion (gcc) specified - retval = malloc (thrown_size + sizeof(__cxa_exception)); + retval = malloc (thrown_size + sizeof(__cxa_refcounted_exception)); if (0 == retval){ std::terminate(); } - memset (retval, 0, sizeof(__cxa_exception)); - return (void *)((unsigned char *)retval + sizeof(__cxa_exception)); + memset (retval, 0, sizeof(__cxa_refcounted_exception)); + return (void *)((unsigned char *)retval + sizeof(__cxa_refcounted_exception)); } extern "C" void __cxa_free_exception(void *vptr) throw(){ - free( (char *)(vptr) - sizeof(__cxa_exception) ); + free( (char *)(vptr) - sizeof(__cxa_refcounted_exception) ); }
Patch for uClibc++ bug #8741 (https://bugs.busybox.net/show_bug.cgi?id=8741) I have discovered that uClibc++ incorrectly implements C++ exception ABI, allocating not enough memory in the "__cxxabiv1::__cxa_allocate_exception": /// code begin /// retval = malloc (thrown_size + sizeof(__cxa_exception)); /// code end /// uClibc++ allocates "thrown_size + sizeof(__cxa_exception)" while stdlibc++ allocates "thrown_size += sizeof (__cxa_refcounted_exception);" since 2009-01-07 (https://gcc.gnu.org/bugzilla/attachment.cgi?id=17047) . The "__cxa_refcounted_exception" is wrapper around "__cxa_exception" and thus bigger than "__cxa_exception" alone. That causes memory corruption (buffer overflow) inside "__cxxabiv1::__cxa_throw" which is not implemented by uClibc++, but implemented by GCC's libsupc++: /// code begin (gcc-5.2.0/libstdc++-v3/libsupc++/eh_throw.cc:69) /// __cxa_refcounted_exception *header = __get_refcounted_exception_header_from_obj (obj); header->referenceCount = 1; /// code end /// In the code above, the "header->referenceCount = 1" writes in memory preceding region allocated for both thrown exception object and exception's structure. The "obj" is pointer to memory allocated by "__cxxabiv1::__cxa_allocate_exception", the "__get_refcounted_exception_header_from_obj (obj)" is defined as: /// code begin /// static inline __cxa_refcounted_exception * __get_refcounted_exception_header_from_obj (void *ptr) { return reinterpret_cast<__cxa_refcounted_exception *>(ptr) - 1; } /// code end /// Thus GCC's libsupc++ expects enough memory allocated preceding exception object to keep structure "__cxa_refcounted_exception", but uClibc++ allocates only "sizeof(__cxa_exception)" bytes before exception object. When binary is compiled for OpenWRT's musl libc standard library, the program crashes with SIGSEGV in the "free" call from "__cxxabiv1::__cxa_free_exception" because musl is very sensitive for memory corruption (musl's malloc stores meta-information about memory region in 8 bytes right before memory chunk itself). When compiled against glibc, the segmentation fault does not happen immediately in the "__cxxabiv1::__cxa_free_exception", but memory corruption still should take place, yielding undefined behavior. Signed-off-by: Ivan Kold <pixus.ru@gmail.com> --- include/unwind-cxx.h | 34 +++++++++++++++++++++++++++++++++- src/eh_alloc.cpp | 8 ++++---- 2 files changed, 37 insertions(+), 5 deletions(-)