From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from hedgehog.birch.relay.mailchannels.net (hedgehog.birch.relay.mailchannels.net [23.83.209.81]) by sourceware.org (Postfix) with ESMTPS id 060C23858D20 for ; Wed, 1 May 2024 17:40:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 060C23858D20 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 060C23858D20 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=23.83.209.81 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1714585212; cv=pass; b=HzVaIhgIeZdmtL9uqwUKO06ZD4jYLj4pZJkQ16kcH1oiES7ovK83n01YE8ZlHf6ya42MByW6q99iyfIsuHiLUvivAU9cwe/60kbRJjQMk44Z+iL2jy7biZXHJH9uxMb7uWsvafydqr4vPwXd170ioKMRRg5HIi79UzafZeMJn9k= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1714585212; c=relaxed/simple; bh=LGTPd/mNKoJo+lC/0gHWA+FlEUtVPnan/isaX3Al9Uc=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=xJ95QR0SsoisodOlHxlCerpu3kVRZmZA8Ia+BeKZlg6g89dszEtgboHat7HhZLwaYhOp4SuP8lYXVbmb0wD1djaH9AqWF/OsxUpl7fKFFn8NU5uiAsYSKBqFEYUomROnxe0y7DoiqQMarl0xoZQ2X1i8l5pRNk/fCWTxv0FnI/Y= 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 AC01110275F; Wed, 1 May 2024 17:40:08 +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 EC6E9102789; Wed, 1 May 2024 17:40:07 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1714585208; a=rsa-sha256; cv=none; b=N8HHduDnYWkhUNG2AiDPqbzpAtJ5vJeByjHUSqqP86g6eGb76CwEeVPdFKBKGdY1Y8Hthx R2ORNg+feMd9G5WNb4y2JxSAT9Wct0XTgEI6sxCs40MQKrvErxgwFf0iLZibTBYA72IZSA dTtTvAM8po1Emhy805811ukmeh6j4j+//1cu0lPYlewg6D4MRjpNZRMw5SP40oEAKTVgi9 DVtZkC75IxdvnDok1/UGG0L6e38tFoOctxqIiYIh1LamgWQ4kpG5Lda+Ky+gsQFz2ygRaY anZKF13FrtgpabmAOxoZ3pqBW+8WsE71+U14Rq4CADgzRuA/Ts4lub2s+5eRcw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1714585208; 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=t8mjTYuqbBZsDruBkMZrdI5L9//nMhNCCB9Al5jUeuc=; b=rITz/wcp4Z+/kdP853t4AMfYNfhgxqlU52iOMD730hHxeSpMXaErglbOz57xZwu5ClSaY2 y4YjnUSDiBeAlAyq3dgF1g1i7D1bj0yywUnNo9Qdlw52ImenTD28d6IJ7dycS9PfgmIJbm UVKz6wZqwyVPVq6AZjSu6g8ddFP87C/IG86Is+OMmmawZK1U5YFxZV4SnJJTY0Z+HO5o+i kHlWv/hwKzbswVaAf3FMmcnUGBxXTaWroNRDxkw/CrD0NFPReMTPzketAXB0y1gN8RzDpZ 9r9MJz+8OP6trj7ybmoUeWmlc2jMRESUgh/7BIcEiS2DHPkEn4u9mlO5chrVqg== ARC-Authentication-Results: i=1; rspamd-5c97bbd7c5-7r97r; 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-Vacuous-Drop: 293e021a4af78dda_1714585208596_690490929 X-MC-Loop-Signature: 1714585208596:1826238455 X-MC-Ingress-Time: 1714585208595 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.123.115.10 (trex/6.9.2); Wed, 01 May 2024 17:40:08 +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 4VV48R3tpFz3n; Wed, 1 May 2024 10:40:07 -0700 (PDT) Message-ID: <7557ee5d-80a9-4692-abe3-10c40c9a4363@sourceware.org> Date: Wed, 1 May 2024 13:40:00 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables To: Adhemerval Zanella , libc-alpha@sourceware.org Cc: Joe Simmons-Talbott References: <20240430192739.1032549-1-adhemerval.zanella@linaro.org> <20240430192739.1032549-5-adhemerval.zanella@linaro.org> Content-Language: en-US From: Siddhesh Poyarekar In-Reply-To: <20240430192739.1032549-5-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: > So tunable with environment variables aliases are also ignored if > glibc.rtld.enable_secure is enabled. The tunable parsing is also > optimized a bit, where the loop that checks each environment variable > only checks for the tunables with aliases instead of all tables. > > Checked on aarch64-linux-gnu and x86_64-linux-gnu. Changing the sorting of the list of tunables ends up breaking internal ABI. It's only a problem that crops up intermittently, e.g. when doing updates where a system may, for a very short time, have a different dynamic linker and libc. How about, instead, something like this: 1. Run through envp twice, first time just for tunables and the second time for envvar aliases. 2. Generate a second, separate array of only the tunables with env aliases for the second envvar alias run. It could be simply: tunable_envalias_t { const char *name; tunable_t *t; }; 3. Skip the second run if enable_secure is set. Thanks, Sid > --- > elf/dl-tunables.c | 34 ++++++-- > elf/tst-tunables-enable_secure.c | 131 +++++++++++++++++++++++++++---- > scripts/gen-tunables.awk | 64 ++++++++++----- > 3 files changed, 189 insertions(+), 40 deletions(-) > > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 63cf8c7ab5..c1a1d1a2e3 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -300,6 +300,10 @@ __tunables_init (char **envp) > if (__libc_enable_secure) > return; > > + /* The tunable with environment variable alias are placed at the start of > + tunable array. */ > + struct tunable_toset_t tunables_env_alias[TUNABLE_NUM_ENV_ALIAS] = { 0 }; > + > while ((envp = get_next_env (envp, &envname, &envval, &prev_envp)) != NULL) > { > /* The environment variable is allocated on the stack by the kernel, so > @@ -311,29 +315,43 @@ __tunables_init (char **envp) > continue; > } > > - for (int i = 0; i < tunables_list_size; i++) > + for (int i = 0; i < TUNABLE_NUM_ENV_ALIAS; i++) > { > tunable_t *cur = &tunable_list[i]; > + const char *name = cur->env_alias; > > - /* Skip over tunables that have either been set already or should be > - skipped. */ > - if (cur->initialized || cur->env_alias[0] == '\0') > + if (name[0] == '\0') > continue; > > - const char *name = cur->env_alias; > - > - /* We have a match. Initialize and move on to the next line. */ > if (tunable_is_name (name, envname)) > { > size_t envvallen = 0; > /* The environment variable is always null-terminated. */ > for (const char *p = envval; *p != '\0'; p++, envvallen++); > > - tunable_initialize (cur, envval, envvallen); > + tunables_env_alias[i] = > + (struct tunable_toset_t) { cur, envval, envvallen }; > break; > } > } > } > + > + /* Check if glibc.rtld.enable_secure was set and skip over the environment > + variables aliases. */ > + if (__libc_enable_secure) > + return; > + > + for (int i = 0; i < TUNABLE_NUM_ENV_ALIAS; i++) > + { > + if (tunables_env_alias[i].t == NULL > + || tunables_env_alias[i].t->initialized) > + continue; > + > + if (!tunable_initialize (tunables_env_alias[i].t, > + tunables_env_alias[i].value, > + tunables_env_alias[i].len)) > + parse_tunable_print_error (&tunables_env_alias[i]); > + } > } > > void > diff --git a/elf/tst-tunables-enable_secure.c b/elf/tst-tunables-enable_secure.c > index f5db1c84e9..d4938a2e5c 100644 > --- a/elf/tst-tunables-enable_secure.c > +++ b/elf/tst-tunables-enable_secure.c > @@ -17,6 +17,7 @@ > . */ > > #include > +#define TUNABLES_INTERNAL 1 > #include > #include > #include > @@ -34,6 +35,8 @@ static int restart; > static const struct test_t > { > const char *env; > + const char *extraenv; > + bool check_multiple; > int32_t expected_malloc_check; > int32_t expected_enable_secure; > } tests[] = > @@ -41,39 +44,124 @@ static const struct test_t > /* Expected tunable format. */ > /* Tunables should be ignored if enable_secure is set. */ > { > - "glibc.malloc.check=2:glibc.rtld.enable_secure=1", > + "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1", > + NULL, > + false, > 0, > 1, > }, > /* Tunables should be ignored if enable_secure is set. */ > { > - "glibc.rtld.enable_secure=1:glibc.malloc.check=2", > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2", > + NULL, > + false, > 0, > 1, > }, > /* Tunables should be set if enable_secure is unset. */ > { > - "glibc.rtld.enable_secure=0:glibc.malloc.check=2", > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2", > + NULL, > + false, > 2, > 0, > }, > + /* Tunables should be ignored if enable_secure is set. */ > + { > + "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1", > + "MALLOC_CHECK_=2", > + false, > + 0, > + 1, > + }, > + /* Same as before, but with enviroment alias prior GLIBC_TUNABLES. */ > + { > + "MALLOC_CHECK_=2", > + "GLIBC_TUNABLES=glibc.malloc.check=2:glibc.rtld.enable_secure=1", > + false, > + 0, > + 1, > + }, > + /* Tunables should be ignored if enable_secure is set. */ > + { > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2", > + "MALLOC_CHECK_=2", > + false, > + 0, > + 1, > + }, > + { > + "MALLOC_CHECK_=2", > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2", > + false, > + 0, > + 1, > + }, > + /* Tunables should be set if enable_secure is unset. */ > + { > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2", > + /* Tunable have precedence over the environment variable. */ > + "MALLOC_CHECK_=1", > + false, > + 2, > + 0, > + }, > + { > + "MALLOC_CHECK_=1", > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=0:glibc.malloc.check=2", > + /* Tunable have precedence over the environment variable. */ > + false, > + 2, > + 0, > + }, > + /* Tunables should be set if enable_secure is unset. */ > + { > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=0", > + /* Tunable have precedence over the environment variable. */ > + "MALLOC_CHECK_=1", > + false, > + 1, > + 0, > + }, > + /* Tunables should be set if enable_secure is unset. */ > + { > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=0", > + /* Tunable have precedence over the environment variable. */ > + "MALLOC_CHECK_=1", > + false, > + 1, > + 0, > + }, > + /* Check with tunables environment variable alias set multiple times. */ > + { > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=1:glibc.malloc.check=2", > + "MALLOC_CHECK_=2", > + true, > + 0, > + 1, > + }, > + /* Tunables should be set if enable_secure is unset. */ > + { > + "GLIBC_TUNABLES=glibc.rtld.enable_secure=0", > + /* Tunable have precedence over the environment variable. */ > + "MALLOC_CHECK_=1", > + true, > + 1, > + 0, > + }, > }; > > static int > handle_restart (int i) > { > if (tests[i].expected_enable_secure == 1) > - { > - TEST_COMPARE (1, __libc_enable_secure); > - } > + TEST_COMPARE (1, __libc_enable_secure); > else > - { > - TEST_COMPARE (tests[i].expected_malloc_check, > - TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL)); > - TEST_COMPARE (tests[i].expected_enable_secure, > - TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, > - NULL)); > - } > + TEST_COMPARE (tests[i].expected_enable_secure, > + TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, > + NULL)); > + TEST_COMPARE (tests[i].expected_malloc_check, > + TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL)); > return 0; > } > > @@ -112,8 +200,23 @@ do_test (int argc, char *argv[]) > > printf ("[%d] Spawned test for %s\n", i, tests[i].env); > setenv ("GLIBC_TUNABLES", tests[i].env, 1); > + > + char *envp[2 + TUNABLE_NUM_ENV_ALIAS + 1] = > + { > + (char *) tests[i].env, > + (char *) tests[i].extraenv, > + NULL, > + }; > + if (tests[i].check_multiple) > + { > + int j; > + for (j=0; j < TUNABLE_NUM_ENV_ALIAS; j++) > + envp[j + 2] = (char *) tests[i].extraenv; > + envp[j + 2] = NULL; > + } > + > struct support_capture_subprocess result > - = support_capture_subprogram (spargv[0], spargv, NULL); > + = support_capture_subprogram (spargv[0], spargv, envp); > support_capture_subprocess_check (&result, "tst-tunables-enable_secure", > 0, sc_allow_stderr); > support_capture_subprocess_free (&result); > diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk > index 9f5336381e..9a18aa6861 100644 > --- a/scripts/gen-tunables.awk > +++ b/scripts/gen-tunables.awk > @@ -14,6 +14,7 @@ BEGIN { > top_ns="" > max_name_len=0 > max_alias_len=0 > + num_env_alias=0 > } > > # Skip over blank lines and comments. > @@ -60,6 +61,8 @@ $1 == "}" { > } > if (!env_alias[top_ns,ns,tunable]) { > env_alias[top_ns,ns,tunable] = "{0}" > + } else { > + num_env_alias = num_env_alias + 1 > } > len = length(top_ns"."ns"."tunable) > if (len > max_name_len) > @@ -125,6 +128,39 @@ $1 == "}" { > } > } > > +function print_tunable_id_t (envfirst) > +{ > + for (tnm in types) { > + split (tnm, indices, SUBSEP); > + t = indices[1]; > + n = indices[2]; > + m = indices[3]; > + if ((envfirst && env_alias[t,n,m] == "{0}") \ > + || (!envfirst && env_alias[t,n,m] != "{0}")) { > + continue; > + } > + printf (" TUNABLE_ENUM_NAME(%s, %s, %s),\n", t, n, m); > + } > +} > + > +function print_tunable_entry (envfirst) > +{ > + for (tnm in types) { > + split (tnm, indices, SUBSEP); > + t = indices[1]; > + n = indices[2]; > + m = indices[3]; > + if ((envfirst && env_alias[t,n,m] == "{0}") \ > + || (!envfirst && env_alias[t,n,m] != "{0}")) { > + continue; > + } > + printf (" {TUNABLE_NAME_S(%s, %s, %s)", t, n, m) > + printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, {%s}, false, %s},\n", > + types[t,n,m], minvals[t,n,m], maxvals[t,n,m], default_val[t,n,m], > + default_val[t,n,m], env_alias[t,n,m]); > + } > +} > + > END { > if (ns != "") { > print "Unterminated namespace. Is a closing brace missing?" > @@ -138,35 +174,27 @@ END { > print "#endif" > print "#include \n" > > + # asort (types) > + > # Now, the enum names > print "\ntypedef enum" > print "{" > - for (tnm in types) { > - split (tnm, indices, SUBSEP); > - t = indices[1]; > - n = indices[2]; > - m = indices[3]; > - printf (" TUNABLE_ENUM_NAME(%s, %s, %s),\n", t, n, m); > - } > + # Make the tunable with environment variables aliases first, their index > + # will be used in the tunable parsing. > + print_tunable_id_t(1) > + print_tunable_id_t(0) > print "} tunable_id_t;\n" > > print "\n#ifdef TUNABLES_INTERNAL" > # Internal definitions. > print "# define TUNABLE_NAME_MAX " (max_name_len + 1) > print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1) > + print "# define TUNABLE_NUM_ENV_ALIAS " (num_env_alias) > print "# include \"dl-tunable-types.h\"" > # Finally, the tunable list. > - print "static tunable_t tunable_list[] attribute_relro = {" > - for (tnm in types) { > - split (tnm, indices, SUBSEP); > - t = indices[1]; > - n = indices[2]; > - m = indices[3]; > - printf (" {TUNABLE_NAME_S(%s, %s, %s)", t, n, m) > - printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, {%s}, false, %s},\n", > - types[t,n,m], minvals[t,n,m], maxvals[t,n,m], default_val[t,n,m], > - default_val[t,n,m], env_alias[t,n,m]); > - } > + print "static tunable_t tunable_list[] attribute_relro __attribute_used__ = {" > + print_tunable_entry(1) > + print_tunable_entry(0) > print "};" > print "#endif" > }