From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69397 invoked by alias); 4 Dec 2018 14:00:00 -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 69340 invoked by uid 89); 4 Dec 2018 13:59:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Forget, H*f:sk:CAKOQZ8, ending, H*i:sk:CAKOQZ8 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; Tue, 04 Dec 2018 13:59:56 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D1A0DC049587; Tue, 4 Dec 2018 13:59:54 +0000 (UTC) Received: from [10.36.117.189] (ovpn-117-189.ams2.redhat.com [10.36.117.189]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F1E881837B; Tue, 4 Dec 2018 13:59:50 +0000 (UTC) To: Ian Lance Taylor Cc: Richard Biener , Jakub Jelinek , matz@gcc.gnu.org, sgayou@redhat.com, Pedro Alves , Tom Tromey , GCC Patches , Binutils , Jason Merrill References: <87muprdko7.fsf@redhat.com> <20181130084211.GX12380@tucnak> <173817ca-0aa0-e1a2-6725-37e079ead545@redhat.com> <20181130140330.GA12380@tucnak> <460cb971-0e21-1e3e-4920-8b3ee7290cf7@redhat.com> From: Nick Clifton Openpgp: preference=signencrypt Subject: Re: RFA/RFC: Add stack recursion limit to libiberty's demangler [v4] Message-ID: <736e8303-b724-f96d-54f5-46bff99fa34d@redhat.com> Date: Tue, 04 Dec 2018 14:00:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------5BFA0DDFE8013575DA082331" X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00186.txt.bz2 This is a multi-part message in MIME format. --------------5BFA0DDFE8013575DA082331 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-length: 1776 Hi Ian, >>> Shouldn't we make it fool-proof by instead introducing a DMGL_NO_RECURSION_LIMIT > You don't need my blessing--I wrote that code ages ago--but I agree > with Richard that in practice it's OK to limit recursion depth by > default. Real symbols have very limited recursion requirements. OK then, here is a fourth revision of the patch. In this version: * The demangler option has been renamed to DMGHL_NO_RECURSE_LIMIT and if the option is not present then the limit is enforced. * I also found another PR that is fixed by the patch, although I had to make sure that the affected code could handle NULL pointers properly afterwards. OK to apply ? Cheers Nick include/ChangeLog 2018-11-29 Nick Clifton * demangle.h (DMGL_NO_RECURSE_LIMIT): Define. (DEMANGLE_RECURSION_LIMIT): Define libiberty/ChangeLog 2018-11-29 Nick Clifton PR 87681 PR 87675 PR 87636 PR 87350 PR 87335 * cp-demangle.h (struct d_info): Add recursion_level field. * cp-demangle.c (d_function_type): Add recursion counter. If the recursion limit is reached and the check is not disabled, then return with a failure result. (cplus_demangle_init_info): Initialise the recursion_level field. (d_demangle_callback): If the recursion limit is enabled, check for a mangled string that is so long that there is not enough stack space for the local arrays. * cplus-dem.c (struct work): Add recursion_level field. (squangle_mop_up): Set the numb and numk fields to zero. (work_stuff_copy_to_from): Handle the case where a btypevec or ktypevec field is NULL. (demangle_nested_args): Add recursion counter. If the recursion limit is not disabled and reached, return with a failure result. --------------5BFA0DDFE8013575DA082331 Content-Type: text/x-patch; name="libiberty-demangler-recursion-limit.4.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="libiberty-demangler-recursion-limit.4.patch" Content-length: 7731 Index: include/demangle.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- include/demangle.h (revision 266771) +++ include/demangle.h (working copy) @@ -68,6 +68,17 @@ /* If none of these are set, use 'current_demangling_style' as the default= . */ #define DMGL_STYLE_MASK (DMGL_AUTO|DMGL_GNU|DMGL_LUCID|DMGL_ARM|DMGL_HP|DM= GL_EDG|DMGL_GNU_V3|DMGL_JAVA|DMGL_GNAT|DMGL_DLANG|DMGL_RUST) =20 +/* Disable a limit on the depth of recursion in mangled strings. + Note if this limit is disabled then stack exhaustion is possible when + demangling pathologically complicated strings. Bug reports about stack + exhaustion when the option is enabled will be rejected. */=20=20 +#define DMGL_NO_RECURSE_LIMIT (1 << 18)=09 + +/* If DMGL_NO_RECURE_LIMIT is not enabled, then this is the value used as + the maximum depth of recursion allowed. It should be enough for any + real-world mangled name. */ +#define DEMANGLE_RECURSION_LIMIT 1024 +=20=20 /* Enumeration of possible demangling styles. =20 Lucid and ARM styles are still kept logically distinct, even though Index: libiberty/cp-demangle.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- libiberty/cp-demangle.c (revision 266771) +++ libiberty/cp-demangle.c (working copy) @@ -2852,21 +2852,35 @@ static struct demangle_component * d_function_type (struct d_info *di) { - struct demangle_component *ret; + struct demangle_component *ret =3D NULL; =20 - if (! d_check_char (di, 'F')) - return NULL; - if (d_peek_char (di) =3D=3D 'Y') + if ((di->options & DMGL_NO_RECURSE_LIMIT) =3D=3D 0) { - /* Function has C linkage. We don't print this information. - FIXME: We should print it in verbose mode. */ - d_advance (di, 1); + if (di->recursion_level > DEMANGLE_RECURSION_LIMIT) + /* FIXME: There ought to be a way to report + that the recursion limit has been reached. */ + return NULL; + + di->recursion_level ++; } - ret =3D d_bare_function_type (di, 1); - ret =3D d_ref_qualifier (di, ret); =20 - if (! d_check_char (di, 'E')) - return NULL; + if (d_check_char (di, 'F')) + { + if (d_peek_char (di) =3D=3D 'Y') + { + /* Function has C linkage. We don't print this information. + FIXME: We should print it in verbose mode. */ + d_advance (di, 1); + } + ret =3D d_bare_function_type (di, 1); + ret =3D d_ref_qualifier (di, ret); +=20=20=20=20=20=20 + if (! d_check_char (di, 'E')) + ret =3D NULL; + } + + if ((di->options & DMGL_NO_RECURSE_LIMIT) =3D=3D 0) + di->recursion_level --; return ret; } =20 @@ -6203,6 +6217,7 @@ di->expansion =3D 0; di->is_expression =3D 0; di->is_conversion =3D 0; + di->recursion_level =3D 0; } =20 /* Internal implementation for the demangler. If MANGLED is a g++ v3 ABI @@ -6242,6 +6257,20 @@ =20 cplus_demangle_init_info (mangled, options, strlen (mangled), &di); =20 + /* PR 87675 - Check for a mangled string that is so long + that we do not have enough stack space to demangle it. */ + if (((options & DMGL_NO_RECURSE_LIMIT) =3D=3D 0) + /* This check is a bit arbitrary, since what we really want to do is= to + compare the sizes of the di.comps and di.subs arrays against the + amount of stack space remaining. But there is no portable way to do + this, so instead we use the recursion limit as a guide to the maximum + size of the arrays. */ + && (unsigned long) di.num_comps > DEMANGLE_RECURSION_LIMIT) + { + /* FIXME: We need a way to indicate that a stack limit has been reac= hed. */ + return 0; + } + { #ifdef CP_DYNAMIC_ARRAYS __extension__ struct demangle_component comps[di.num_comps]; Index: libiberty/cp-demangle.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- libiberty/cp-demangle.h (revision 266771) +++ libiberty/cp-demangle.h (working copy) @@ -122,6 +122,9 @@ /* Non-zero if we are parsing the type operand of a conversion operator, but not when in an expression. */ int is_conversion; + /* If DMGL_NO_RECURSE_LIMIT is not active then this is set to + the current recursion level. */ + unsigned int recursion_level; }; =20 /* To avoid running past the ending '\0', don't: Index: libiberty/cplus-dem.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- libiberty/cplus-dem.c (revision 266771) +++ libiberty/cplus-dem.c (working copy) @@ -146,6 +146,7 @@ int *proctypevec; /* Indices of currently processed remembered typev= ecs. */ int proctypevec_size; int nproctypes; + unsigned int recursion_level; }; =20 #define PRINT_ANSI_QUALIFIERS (work -> options & DMGL_ANSI) @@ -1292,6 +1293,7 @@ free ((char *) work -> btypevec); work->btypevec =3D NULL; work->bsize =3D 0; + work->numb =3D 0; } if (work -> ktypevec !=3D NULL) { @@ -1298,6 +1300,7 @@ free ((char *) work -> ktypevec); work->ktypevec =3D NULL; work->ksize =3D 0; + work->numk =3D 0; } } =20 @@ -1331,8 +1334,15 @@ =20 for (i =3D 0; i < from->numk; i++) { - int len =3D strlen (from->ktypevec[i]) + 1; + int len; =20 + if (from->ktypevec[i] =3D=3D NULL) + { + to->ktypevec[i] =3D NULL; + continue; + } + + len =3D strlen (from->ktypevec[i]) + 1; to->ktypevec[i] =3D XNEWVEC (char, len); memcpy (to->ktypevec[i], from->ktypevec[i], len); } @@ -1342,8 +1352,15 @@ =20 for (i =3D 0; i < from->numb; i++) { - int len =3D strlen (from->btypevec[i]) + 1; + int len; =20 + if (from->btypevec[i] =3D=3D NULL) + { + to->btypevec[i] =3D NULL; + continue; + } + + len =3D strlen (from->btypevec[i]) + 1; to->btypevec[i] =3D XNEWVEC (char , len); memcpy (to->btypevec[i], from->btypevec[i], len); } @@ -1401,6 +1418,7 @@ =20 free ((char*) work->tmpl_argvec); work->tmpl_argvec =3D NULL; + work->ntmpl_args =3D 0; } if (work->previous_argument) { @@ -4478,6 +4496,7 @@ } =20 /* Lose all the info related to B and K type codes. */ + static void forget_B_and_K_types (struct work_stuff *work) { @@ -4503,6 +4522,7 @@ } } } + /* Forget the remembered types, but not the type vector itself. */ =20 static void @@ -4693,10 +4713,21 @@ demangle_nested_args (struct work_stuff *work, const char **mangled, string *declp) { + static unsigned long recursion_level =3D 0; string* saved_previous_argument; int result; int saved_nrepeats; =20 + if ((work->options & DMGL_NO_RECURSE_LIMIT) =3D=3D 0 + && work->recursion_level > DEMANGLE_RECURSION_LIMIT) + { + /* FIXME: There ought to be a way to report that the recursion limit + has been reached. */ + return 0; + } + + recursion_level ++; + /* The G++ name-mangling algorithm does not remember types on nested argument lists, unless -fsquangling is used, and in that case the type vector updated by remember_type is not used. So, we turn @@ -4723,6 +4754,7 @@ --work->forgetting_types; work->nrepeats =3D saved_nrepeats; =20 + --recursion_level; return result; } =20 --------------5BFA0DDFE8013575DA082331--