libiberty: Limit demangler maximum d_print_comp recursion call depth.

Message ID 1492554228-9972-1-git-send-email-mark@klomp.org
State New
Headers show

Commit Message

Mark Wielaard April 18, 2017, 10:23 p.m.
The fix for PR demangler/70909 and 67264 (endless demangler recursion)
catches when a demangle_component is printed in a cycle. But that doesn't
protect the call stack blowing up from non-cyclic nested types printed
recursively through d_print_comp. This can happen by a (very) long mangled
string that simply creates a very deep pointer or qualifier chain. Limit
the recursive d_print_comp call depth for a d_print_info to 1K nested
types.

libiberty/ChangeLog:

        * cp-demangle.c (MAX_RECURSION_COUNT): New constant.
        (struct d_print_info): Add recursion field.
        (d_print_init): Initialize recursion.
        (d_print_comp): Check and update d_print_info recursion depth.

Comments

Ian Lance Taylor via gcc-patches April 18, 2017, 10:40 p.m. | #1
On Tue, Apr 18, 2017 at 3:23 PM, Mark Wielaard <mark@klomp.org> wrote:
> The fix for PR demangler/70909 and 67264 (endless demangler recursion)
> catches when a demangle_component is printed in a cycle. But that doesn't
> protect the call stack blowing up from non-cyclic nested types printed
> recursively through d_print_comp. This can happen by a (very) long mangled
> string that simply creates a very deep pointer or qualifier chain. Limit
> the recursive d_print_comp call depth for a d_print_info to 1K nested
> types.
>
> libiberty/ChangeLog:
>
>         * cp-demangle.c (MAX_RECURSION_COUNT): New constant.
>         (struct d_print_info): Add recursion field.
>         (d_print_init): Initialize recursion.
>         (d_print_comp): Check and update d_print_info recursion depth.

I'm probably missing something, but this kind of seems like an
arbitrary limit.  It's possible to imagine a rather unlikely valid
symbol that will no longer be demangled.  Why do we want to do this?
What bug are we fixing?

Ian
Mark Wielaard April 18, 2017, 11:03 p.m. | #2
On Tue, Apr 18, 2017 at 03:40:05PM -0700, Ian Lance Taylor wrote:
> On Tue, Apr 18, 2017 at 3:23 PM, Mark Wielaard <mark@klomp.org> wrote:
> > The fix for PR demangler/70909 and 67264 (endless demangler recursion)
> > catches when a demangle_component is printed in a cycle. But that doesn't
> > protect the call stack blowing up from non-cyclic nested types printed
> > recursively through d_print_comp. This can happen by a (very) long mangled
> > string that simply creates a very deep pointer or qualifier chain. Limit
> > the recursive d_print_comp call depth for a d_print_info to 1K nested
> > types.
> >
> > libiberty/ChangeLog:
> >
> >         * cp-demangle.c (MAX_RECURSION_COUNT): New constant.
> >         (struct d_print_info): Add recursion field.
> >         (d_print_init): Initialize recursion.
> >         (d_print_comp): Check and update d_print_info recursion depth.
> 
> I'm probably missing something, but this kind of seems like an
> arbitrary limit.  It's possible to imagine a rather unlikely valid
> symbol that will no longer be demangled.  Why do we want to do this?
> What bug are we fixing?

It is an arbitrary limit and I am happy to change it if it is unrealistic.
I thought 1K was small enough that if we hit it we wouldn't have blown up
the call stack yet. But big enough that it is unlikely that it would be a
valid symbol (with that large a number of nested component types). The bug
we fix with this is a program trying to demangle a string that looks like
e.g. _Z3fnGGGGGGOGGGGGGGGGGGGGGGGGGG.... crashing because of stack overflow.

Cheers,

Mark
Ian Lance Taylor via gcc-patches April 18, 2017, 11:24 p.m. | #3
On Tue, Apr 18, 2017 at 4:03 PM, Mark Wielaard <mark@klomp.org> wrote:
> On Tue, Apr 18, 2017 at 03:40:05PM -0700, Ian Lance Taylor wrote:
>> On Tue, Apr 18, 2017 at 3:23 PM, Mark Wielaard <mark@klomp.org> wrote:
>> > The fix for PR demangler/70909 and 67264 (endless demangler recursion)
>> > catches when a demangle_component is printed in a cycle. But that doesn't
>> > protect the call stack blowing up from non-cyclic nested types printed
>> > recursively through d_print_comp. This can happen by a (very) long mangled
>> > string that simply creates a very deep pointer or qualifier chain. Limit
>> > the recursive d_print_comp call depth for a d_print_info to 1K nested
>> > types.
>> >
>> > libiberty/ChangeLog:
>> >
>> >         * cp-demangle.c (MAX_RECURSION_COUNT): New constant.
>> >         (struct d_print_info): Add recursion field.
>> >         (d_print_init): Initialize recursion.
>> >         (d_print_comp): Check and update d_print_info recursion depth.
>>
>> I'm probably missing something, but this kind of seems like an
>> arbitrary limit.  It's possible to imagine a rather unlikely valid
>> symbol that will no longer be demangled.  Why do we want to do this?
>> What bug are we fixing?
>
> It is an arbitrary limit and I am happy to change it if it is unrealistic.
> I thought 1K was small enough that if we hit it we wouldn't have blown up
> the call stack yet. But big enough that it is unlikely that it would be a
> valid symbol (with that large a number of nested component types). The bug
> we fix with this is a program trying to demangle a string that looks like
> e.g. _Z3fnGGGGGGOGGGGGGGGGGGGGGGGGGG.... crashing because of stack overflow.

Hmmm, well, OK for stage 1, I guess.

Ian

Patch

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index aeff7a7..e1db900 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -319,6 +319,9 @@  struct d_info_checkpoint
   int expansion;
 };
 
+/* Maximum number of times d_print_comp may be called recursively.  */
+#define MAX_RECURSION_COUNT 1024
+
 enum { D_PRINT_BUFFER_LENGTH = 256 };
 struct d_print_info
 {
@@ -341,6 +344,9 @@  struct d_print_info
   struct d_print_mod *modifiers;
   /* Set to 1 if we saw a demangling error.  */
   int demangle_failure;
+  /* Number of times d_print_comp was recursively called.  Should not
+     be bigger than MAX_RECURSION_COUNT.  */
+  int recursion;
   /* Non-zero if we're printing a lambda argument.  A template
      parameter reference actually means 'auto'.  */
   int is_lambda_arg;
@@ -4151,6 +4157,7 @@  d_print_init (struct d_print_info *dpi, demangle_callbackref callback,
   dpi->opaque = opaque;
 
   dpi->demangle_failure = 0;
+  dpi->recursion = 0;
   dpi->is_lambda_arg = 0;
 
   dpi->component_stack = NULL;
@@ -5685,13 +5692,14 @@  d_print_comp (struct d_print_info *dpi, int options,
 	      struct demangle_component *dc)
 {
   struct d_component_stack self;
-  if (dc == NULL || dc->d_printing > 1)
+  if (dc == NULL || dc->d_printing > 1 || dpi->recursion > MAX_RECURSION_COUNT)
     {
       d_print_error (dpi);
       return;
     }
-  else
-    dc->d_printing++;
+
+  dc->d_printing++;
+  dpi->recursion++;
 
   self.dc = dc;
   self.parent = dpi->component_stack;
@@ -5701,6 +5709,7 @@  d_print_comp (struct d_print_info *dpi, int options,
 
   dpi->component_stack = self.parent;
   dc->d_printing--;
+  dpi->recursion--;
 }
 
 /* Print a Java dentifier.  For Java we try to handle encoded extended