| Message ID | 1492554228-9972-1-git-send-email-mark@klomp.org |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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.