From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 73543 invoked by alias); 10 Dec 2018 15:35:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 73493 invoked by uid 89); 10 Dec 2018 15:35:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=1.3 required=5.0 tests=BAYES_50,KAM_NUMSUBJECT,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=sk:DEMANGL, H*f:sk:fca558b, H*f:sk:B6beozo, H*f:sk:2f4c983 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 10 Dec 2018 15:35:44 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5FB3681DED; Mon, 10 Dec 2018 15:35:43 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-117-214.ams2.redhat.com [10.36.117.214]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EF43419C7C; Mon, 10 Dec 2018 15:35:42 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id wBAFZeDC022667; Mon, 10 Dec 2018 16:35:40 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id wBAFZc4h022666; Mon, 10 Dec 2018 16:35:38 +0100 Date: Mon, 10 Dec 2018 15:35:00 -0000 From: Jakub Jelinek To: Nick Clifton Cc: Michael Matz , Ian Lance Taylor , "H.J. Lu" , Pedro Alves , Richard Guenther , sgayou@redhat.com, Tom Tromey , GCC Patches , Binutils , Jason Merrill Subject: Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536 Message-ID: <20181210153538.GC12380@tucnak> Reply-To: Jakub Jelinek References: <2f4c983b-494f-93ba-d6c6-1fe0a9730a76@redhat.com> <9e739bbc-38a2-c3b1-3b2b-f8de4b755d47@redhat.com> <20181210151846.GB12380@tucnak> <0af34ffd-e894-2803-7c4e-eac4d9ffb385@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0af34ffd-e894-2803-7c4e-eac4d9ffb385@redhat.com> User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00587.txt.bz2 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