This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]


On Mon, Dec 10, 2018 at 7:35 AM Jakub Jelinek <> wrote:
> On Mon, Dec 10, 2018 at 03:26:18PM +0000, Nick Clifton wrote:
> > >> My current suggestion
> > >> is to raise the limit to 2048, which allows the libiberty patch to
> > >> pass.  But do you have a feel for how much is a realistic limit ?
> > >
> > > For recursion limit I think that is fine.
> > > For just stack size limit, I think it is extremely small.
> > > I see that in the function it allocates on 64-bit 24 bytes times
> > > num_comps using alloca, so 48 bytes per character in the mangled name,
> > > and a pointer for each character in the mangled name.
> > > That is 112KB per 2048 bytes long mangled name.
> > >
> > > Dunno how much stack can we expect to be usable.
> >
> > Currently the patched libiberty uses the DEMANGLE_RECURSION_LIMIT value
> > in two ways.  The first is as a limit on the number of levels of recursion
> > of specific functions inside the demangler.  The second is as a check on
> > the number of component structures that will be allocated on the stack.
> > (See cp-demangle.c:d_demangle_callback).  One of the CVEs that I was checking
> > was triggering stack exhaustion this way, which is why I added the check.
> >
> > There is at least one other function where a similar stack allocation
> > happens (cplus_demangle_print_callback) but I was not sure if this could
> > be triggered with the other limits in place, and I did not have a reproducer
> > that touched it, so I left it alone.
> I think it is a bad idea to use the same macro and value for both the
> recursion limit and essentially stack limit.  For the latter, it should
> actually compute expected stack size
> (i.e. di.num_comps * sizeof (*di.comps) + di.num_subs * sizeof (*di.subs))
> and rather than just giving up should say that memory up to 64KB this
> way will be handled via alloca, more through say mmap (I'd avoid malloc
> because some users are using these APIs in cases where malloc isn't usable).
> And have only much larger limit, like say 1MB for these arrays on which to
> give up.

That makes sense.

We've wanted to avoid malloc in this code because some programs call
the demangler from a signal handler.  But using mmap should be fine in


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]