From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from slateblue.cherry.relay.mailchannels.net (slateblue.cherry.relay.mailchannels.net [23.83.223.168]) by sourceware.org (Postfix) with ESMTPS id 4B7963858D20 for ; Wed, 1 May 2024 16:30:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4B7963858D20 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=sourceware.org Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=sourceware.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4B7963858D20 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=23.83.223.168 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1714581057; cv=pass; b=UeQma2IoYTSuNUXvTUnuaKgRuM1buXfTT+/wv4jH+5kOQENVAbCUNXiFfAGJWHxAenTES2voaQylJ/LRm/aIIMPlp7EytPRI2XCNYybh+w08P5a0AEuvjPCDagQTIkaK+s0dAxQakuDR0J5rrTDj6VNiAypcp9JWkEqPF3U3J50= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1714581057; c=relaxed/simple; bh=1JJCYrX/aoQODjauRB6LmcCYLNKzucu4eQD6x9RSslQ=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=r29BZEevV8VLQJc/ndCHU82UxmljZzt0ATgpGILoSVpNuLAdKZgwBqwsOzZzAsCwlcsdhDygMkaKRR7KT5GWsEwh3G9EsG9KPLPvLIRDosalHJyAGdEEtiLMO72fPp/YubDupxub1zQv53eDX3wu/+VAfx6mF1VXcVwcnvRDc+s= ARC-Authentication-Results: i=2; server2.sourceware.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 52BE5542E35; Wed, 1 May 2024 16:30:54 +0000 (UTC) Received: from pdx1-sub0-mail-a208.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 7ED8A5420A6; Wed, 1 May 2024 16:30:52 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1714581052; a=rsa-sha256; cv=none; b=rEfJHND1PFVVAr8P1q8wBb5/0o7/aR81cMLnKLCvuhmUor5xs2l2kswW+Bdy/jrnJlkhCd eDKXkzOMWENwhjHI1YykdVNky1opiFcoOEJRemowujCYKB9BUW1Edekmgux8t5jYZpB3kr Bdd0zYYE4WPPXee7++OIbgxGkdffYMTDDSj28BgmzDr0CBVSYdKLZ7fa1dyJGaP3+jpJMv vL1g8Ej6rh8Y72w0dB7HWQQsBORaWHnBTjQuW1tWJGDlrVmms2Iy+sRgkM9IK62xD9by7i 4DvTFfLe5a9gVbvRaDtaFvITfwxGo0aLFcLEJgdOB4qy0TdkNG3Q5dCZyeOUwA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1714581052; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oEdBuddc4F1RjFrdFi3WrW4qvCLyfup5ZYKByYow/MI=; b=VUGusUWxr6kEjmB3jtH0HToY1Pt1PPpXWt6lT+3huYU0rt4TTgln9DZMzcvlrslL97xYBQ gWBh71ykg8QNYLT+x11vf5u4XHgAFag6MHMvbyEcv3bhl1SuV54zac/W4mZ5OL1iIWVDyb 6L5CG6w/dueX8PU9piwGrVj7PcI07cuQAqSm6coIBwuQ/7FjzXvUc6al8XuyLbBr3Fbd8Y VZCY6/dCoTDfdEQQgjElcLTnIAmAjqLWtie9APa6X0SwhDV10ACg+x7MXQOrW8c8enC7vV PwezIZFwmU3wH2l7pOlZe8BYqom3m4mHgXWiYswcFKybCpo13R5dfJe2CZTlcA== ARC-Authentication-Results: i=1; rspamd-5c97bbd7c5-ktckl; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@sourceware.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Imminent-Cure: 4b6d4e9640543d7b_1714581053929_3774206613 X-MC-Loop-Signature: 1714581053929:1501526643 X-MC-Ingress-Time: 1714581053928 Received: from pdx1-sub0-mail-a208.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.127.52.150 (trex/6.9.2); Wed, 01 May 2024 16:30:53 +0000 Received: from [192.168.0.182] (bras-base-toroon4859w-grc-71-142-198-113-116.dsl.bell.ca [142.198.113.116]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a208.dreamhost.com (Postfix) with ESMTPSA id 4VV2cW5WYyz3n; Wed, 1 May 2024 09:30:51 -0700 (PDT) Message-ID: <3190de63-d1ec-4d84-bf01-56b265c12f14@sourceware.org> Date: Wed, 1 May 2024 12:30:49 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/4] elf: Only process multiple tunable once (BZ 31686) To: Adhemerval Zanella , libc-alpha@sourceware.org Cc: Joe Simmons-Talbott , Yuto Maeda , Yutaro Shimizu References: <20240430192739.1032549-1-adhemerval.zanella@linaro.org> <20240430192739.1032549-2-adhemerval.zanella@linaro.org> Content-Language: en-US From: Siddhesh Poyarekar In-Reply-To: <20240430192739.1032549-2-adhemerval.zanella@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-1173.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_SOFTFAIL,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2024-04-30 15:25, Adhemerval Zanella wrote: > The parse_tunables_string is a tunable entry on the 'tunable_list' list > to be set later without checking if the entry is already present. If > leads to a stack overflow if the tunable is set multiple times, > for instance: > > GLIBC_TUNABLES=glibc.malloc.check=2:... (repeat over the number of > total support for different tunable). > > Instead, use the index of the tunable list to get the expected tunable > entry. Since now the initial list is zero-initialized, the compiler > might emit an extra memset and this requires some minor adjustment > on some ports. > > Checked on x86_64-linux-gnu and aarch64-linux-gnu. > > Reported-by: Yuto Maeda Please also credit Yutaro Shimizu in reporters. > --- > elf/dl-tunables.c | 30 ++++++----- > elf/tst-tunables.c | 59 +++++++++++++++++++++- > sysdeps/aarch64/multiarch/memset_generic.S | 4 ++ > sysdeps/sparc/sparc64/rtld-memset.c | 3 ++ > 4 files changed, 82 insertions(+), 14 deletions(-) > > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index d3ccd2ecd4..1db80e0f92 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -32,6 +32,7 @@ > #include > #include > #include > +#include > > #define TUNABLES_INTERNAL 1 > #include "dl-tunables.h" > @@ -221,8 +222,7 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) > > if (tunable_is_name (cur->name, name)) > { > - tunables[ntunables++] = > - (struct tunable_toset_t) { cur, value, p - value }; > + tunables[i] = (struct tunable_toset_t) { cur, value, p - value }; Uninitialized tunables stay as NULL, tunables appearing later in the string will persist in case of duplicates. OK. > > /* Ignore tunables if enable_secure is set */ > if (tunable_is_name ("glibc.rtld.enable_secure", name)) > @@ -245,23 +245,27 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) > static void > parse_tunables (const char *valstring) > { > - struct tunable_toset_t tunables[tunables_list_size]; > - int ntunables = parse_tunables_string (valstring, tunables); > - if (ntunables == -1) > + struct tunable_toset_t tunables[tunables_list_size] = { 0 }; zero-initialized, so tunables[i].t is NULL unless set above. OK. > + if (parse_tunables_string (valstring, tunables) == -1) > { > _dl_error_printf ( > "WARNING: ld.so: invalid GLIBC_TUNABLES `%s': ignored.\n", valstring); > return; > } > > - for (int i = 0; i < ntunables; i++) > - if (!tunable_initialize (tunables[i].t, tunables[i].value, > - tunables[i].len)) > - _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' " > - "for option `%s': ignored.\n", > - (int) tunables[i].len, > - tunables[i].value, > - tunables[i].t->name); > + for (int i = 0; i < tunables_list_size; i++) > + { > + if (tunables[i].t == NULL) > + continue; Skip over NULLs, initializing only those that are set. OK. > + > + if (!tunable_initialize (tunables[i].t, tunables[i].value, > + tunables[i].len)) > + _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' " > + "for option `%s': ignored.\n", > + (int) tunables[i].len, > + tunables[i].value, > + tunables[i].t->name); > + } > } > > /* Initialize the tunables list from the environment. For now we only use the > diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c > index 095b5c81d9..ce5f62f777 100644 > --- a/elf/tst-tunables.c > +++ b/elf/tst-tunables.c > @@ -17,6 +17,7 @@ > . */ > > #include > +#define TUNABLES_INTERNAL 1 > #include > #include > #include > @@ -24,12 +25,13 @@ > #include > #include > #include > +#include > > static int restart; > #define CMDLINE_OPTIONS \ > { "restart", no_argument, &restart, 1 }, > > -static const struct test_t > +static struct test_t > { > const char *name; > const char *value; > @@ -284,6 +286,29 @@ static const struct test_t > 0, > 0, > }, > + /* Also check for repeated tunables with a count larger than the total number > + of tunables. */ > + { > + "GLIBC_TUNABLES", > + NULL, > + 2, > + 0, > + 0, > + }, > + { > + "GLIBC_TUNABLES", > + NULL, > + 1, > + 0, > + 0, > + }, > + { > + "GLIBC_TUNABLES", > + NULL, > + 0, > + 0, > + 0, > + }, > }; > > static int > @@ -316,6 +341,7 @@ do_test (int argc, char *argv[]) > > char nteststr[INT_BUFSIZE_BOUND (int)]; > > + Spurious newline. > char *spargv[10]; > { > int i = 0; > @@ -327,6 +353,37 @@ do_test (int argc, char *argv[]) > spargv[i] = NULL; > } > > + /* Create a tunable line with the duplicate values with a total number > + larger than the different number of tunables. */ > + { > + enum { tunables_list_size = array_length (tunable_list) }; > + const char *value = ""; > + for (int i = 0; i < tunables_list_size; i++) > + value = xasprintf ("%sglibc.malloc.check=2%c", > + value, > + i == (tunables_list_size - 1) ? '\0' : ':'); > + tests[33].value = value; > + } > + /* Same as before, but the last tunable vallues is differen than the > + rest. */ > + { > + enum { tunables_list_size = array_length (tunable_list) }; > + const char *value = ""; > + for (int i = 0; i < tunables_list_size - 1; i++) > + value = xasprintf ("%sglibc.malloc.check=2:", value); > + value = xasprintf ("%sglibc.malloc.check=1", value); > + tests[34].value = value; > + } > + /* Same as before, but with an invalid last entry. */ > + { > + enum { tunables_list_size = array_length (tunable_list) }; > + const char *value = ""; > + for (int i = 0; i < tunables_list_size - 1; i++) > + value = xasprintf ("%sglibc.malloc.check=2:", value); > + value = xasprintf ("%sglibc.malloc.check=1=1", value); > + tests[35].value = value; > + } > + > for (int i = 0; i < array_length (tests); i++) > { > snprintf (nteststr, sizeof nteststr, "%d", i); > diff --git a/sysdeps/aarch64/multiarch/memset_generic.S b/sysdeps/aarch64/multiarch/memset_generic.S > index 81748bdbce..e125a5ed85 100644 > --- a/sysdeps/aarch64/multiarch/memset_generic.S > +++ b/sysdeps/aarch64/multiarch/memset_generic.S > @@ -33,3 +33,7 @@ > #endif > > #include <../memset.S> > + > +#if IS_IN (rtld) > +strong_alias (memset, __memset_generic) > +#endif The extra memset you mentioned. OK. > diff --git a/sysdeps/sparc/sparc64/rtld-memset.c b/sysdeps/sparc/sparc64/rtld-memset.c > index 55f3835790..a19202a620 100644 > --- a/sysdeps/sparc/sparc64/rtld-memset.c > +++ b/sysdeps/sparc/sparc64/rtld-memset.c > @@ -1 +1,4 @@ > #include > +#if IS_IN(rtld) > +strong_alias (memset, __memset_ultra1) > +#endif Likewise. Thanks, Sid