This is the mail archive of the binutils@sourceware.org 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]

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536


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.

	Jakub


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