uClibc++: any throw statement causes memory corruption

Submitted by Ivan Koldaev on March 4, 2016, 7:48 p.m.

Details

Message ID CAPNORr-c-oe5Bw_d-ZE9k4TcD6pnS7QQuz0Usej_oFdUKyRoPw@mail.gmail.com
State New
Headers show

Commit Message

Ivan Koldaev March 4, 2016, 7:48 p.m.
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(-)

Comments

Rob Landley March 11, 2016, 9:41 p.m.
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
Ivan Koldaev March 11, 2016, 11:18 p.m.
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.
aldot March 12, 2016, 5:20 p.m.
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,
Rob Landley March 12, 2016, 5:59 p.m.
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

Patch hide | download patch | download mbox

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) );
 }