This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Nick Clifton <nickc at redhat dot com>
- Cc: Michael Matz <matz at suse dot de>, Ian Lance Taylor <iant at google dot com>, "H.J. Lu" <hjl dot tools at gmail dot com>, Pedro Alves <palves at redhat dot com>, Richard Guenther <richard dot guenther at gmail dot com>, sgayou at redhat dot com, Tom Tromey <tom at tromey dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Binutils <binutils at sourceware dot org>, Jason Merrill <jason at redhat dot com>
- Date: Mon, 10 Dec 2018 16:35:38 +0100
- Subject: Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536
- References: <email@example.com> <firstname.lastname@example.org> <email@example.com> <CAKOQZ8y=B6beozokJ2tdAAkVDVue08ogehMP7TAXvrPzdz9MuQ@mail.gmail.com> <CAMe9rOomd2E3C03CxTXyTRkq6HG32OX+rbMPS3y6dcEWmwaMYg@mail.gmail.com> <CAMe9rOokMpaAUFk0rcYTTUQTQhEMn-VQetXfiDTDXYdTXZEJTA@mail.gmail.com> <alpine.LSU.firstname.lastname@example.org> <email@example.com> <20181210151846.GB12380@tucnak> <firstname.lastname@example.org>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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