From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from toucan.tulip.relay.mailchannels.net (toucan.tulip.relay.mailchannels.net [23.83.218.254]) by sourceware.org (Postfix) with ESMTPS id 1A0C43858D1E for ; Tue, 7 May 2024 14:33:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1A0C43858D1E 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 1A0C43858D1E Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=23.83.218.254 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1715092424; cv=pass; b=oDFxMW5FZST4sUlNu0KXEF4Hcz77FBnx4WJciK3IwCliZCRY7SIDnOVw9Bicch8mj8brx1n8WocsPo1KrccTRILyglcy1i+/K4rexKxGBHM249h4XFuxLI65c3ogAmxNxy2qxi97uSNSnxISfVWZzXGJXWTjr3nGmFKhwzJXyjY= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1715092424; c=relaxed/simple; bh=HuMHIw3pg6G6lQuLBVWHy0M4zyYqO3AbezpR2O+QSkI=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=V9bSQvW7wfeMVwbQ2tk+pEVOwvv3biTAUu39ILVMQh+JA/r+WIcpoITcykTsxHkNhMd5PfsuP2TiAM8g9Q4EPu6HwliJWedZr7PNlxmosXbwordmQvR+L5r1vJnr3+RE2u0Mm7uhOKQAxxOKNqK3jbVU9WBg9RkTGH1lrimbc3M= 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 E02BA4C3939; Tue, 7 May 2024 14:33:28 +0000 (UTC) Received: from pdx1-sub0-mail-a203.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 033184C1BDD; Tue, 7 May 2024 14:33:28 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1715092408; a=rsa-sha256; cv=none; b=RHEt7FpITu1FqnD1NtObtzpiHz9gfaBeeQJSKTswi/Y+J35sGgMhp6vPwUrYYL640bporD 8mKFxYI1vqe3JY+X/6+rBfx7cFZjsSRf73ibXDbKLWBKiC0RPCbj0jNILbCKEaa+pqrmSu z5UUR7nTrnbHzAAv3sccUo4LAs86r28eHrjtkltRPWWzBrouCmG1DkO72x6qh3qC9n9Cf+ 1oXjR8iSy78GPhZj4DpPqZfxBBeG8yB9eccTx8YgtcH247GCtdL4UhKxDTKzAOw0l0vq57 8flZOLuRriYvlR0m3nl7ThqXCecLBUaCpsr7UbbxtY8cHx5lO5UOgbK0Jb0D7A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1715092408; 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=UXVvsjPauVF3PKls9/PcHx/d9THq6uhlOV69QSBjxyo=; b=9xZ90WnI83yC/irNLsdtxR7rgBavNbR5CdCN/tA7ZQEZgELd8I5UJuYe0EwvgLDZEcT2AR TOcAJ+LpeGMSpZuNktUHtsV5qEyijcDV32OdK1bEotX93cnW9TP77HcxKQIfcb3e2BenzD qTwgeA1x4X2YIU0mvdsu/IcQ9n2fJ9dD3PHn6x3xZE76PirRYChz2Z7w1jRvA8DlWEUZ4g T8LL8ACGRSDC0GSixikvvog7NsYb2UOT/CeaVF8XFNOm32S12M1Uf0xI7y07wjeZ496Ro6 14ru5XXTQ7WuWkwGrfPa+6EAAItRI6Y7y7aPhGCVk481D6HjiRyc6vyTOCTDpg== ARC-Authentication-Results: i=1; rspamd-68bbddc7f5-9knrk; 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-Cooing-Chief: 345146155d6e7822_1715092408487_3479216997 X-MC-Loop-Signature: 1715092408487:1556534816 X-MC-Ingress-Time: 1715092408486 Received: from pdx1-sub0-mail-a203.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.120.224.30 (trex/6.9.2); Tue, 07 May 2024 14:33:28 +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-a203.dreamhost.com (Postfix) with ESMTPSA id 4VYgkH405fzB9; Tue, 7 May 2024 07:33:27 -0700 (PDT) Message-ID: <487b1377-b5b0-4b4b-b843-2a893b39af90@sourceware.org> Date: Tue, 7 May 2024 10:33:19 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables To: Adhemerval Zanella , libc-alpha@sourceware.org Cc: Joe Simmons-Talbott References: <20240506161955.1570278-1-adhemerval.zanella@linaro.org> <20240506161955.1570278-5-adhemerval.zanella@linaro.org> Content-Language: en-US From: Siddhesh Poyarekar In-Reply-To: <20240506161955.1570278-5-adhemerval.zanella@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-1172.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,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-05-06 12:18, Adhemerval Zanella wrote: > 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. > --- LGTM. Reviewed-by: Siddhesh Poyarekar > elf/dl-tunables.c | 36 ++++++-- > elf/tst-tunables-enable_secure.c | 136 +++++++++++++++++++++++++++---- > scripts/gen-tunables.awk | 16 +++- > 3 files changed, 164 insertions(+), 24 deletions(-) > > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 63cf8c7ab5..147cc4cf23 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -300,6 +300,9 @@ __tunables_init (char **envp) > if (__libc_enable_secure) > return; > > + enum { tunable_num_env_alias = array_length (tunable_env_alias_list) }; > + 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 +314,44 @@ __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]; > + tunable_t *cur = &tunable_list[tunable_env_alias_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++) > + { > + /* Skip over tunables that have either been set or already initialized. */ > + 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..99a065e8db 100644 > --- a/elf/tst-tunables-enable_secure.c > +++ b/elf/tst-tunables-enable_secure.c > @@ -17,6 +17,10 @@ > . */ > > #include > +/* The test uses the tunable_env_alias_list size, which is only exported for > + ld.so. This will result in a copy of tunable_list and > + tunable_env_alias_list, which is ununsed by the test itself. */ > +#define TUNABLES_INTERNAL 1 > #include > #include > #include > @@ -34,6 +38,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 +47,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; > } > > @@ -106,14 +197,31 @@ do_test (int argc, char *argv[]) > spargv[i] = NULL; > } > > + enum { tunable_num_env_alias = array_length (tunable_env_alias_list) }; > + > for (int i = 0; i < array_length (tests); i++) > { > snprintf (nteststr, sizeof nteststr, "%d", i); > > 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..fc3b41376f 100644 > --- a/scripts/gen-tunables.awk > +++ b/scripts/gen-tunables.awk > @@ -156,7 +156,7 @@ END { > print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1) > print "# include \"dl-tunable-types.h\"" > # Finally, the tunable list. > - print "static tunable_t tunable_list[] attribute_relro = {" > + print "static tunable_t tunable_list[] attribute_relro __attribute_used__ = {" > for (tnm in types) { > split (tnm, indices, SUBSEP); > t = indices[1]; > @@ -168,5 +168,19 @@ END { > default_val[t,n,m], env_alias[t,n,m]); > } > print "};" > + > + # Map of tunable with environment variables aliases used during parsing. */ > + print "\nstatic const tunable_id_t tunable_env_alias_list[] =" > + printf "{\n" > + for (tnm in types) { > + split (tnm, indices, SUBSEP); > + t = indices[1]; > + n = indices[2]; > + m = indices[3]; > + if (env_alias[t,n,m] != "{0}") { > + printf (" TUNABLE_ENUM_NAME(%s, %s, %s),\n", t, n, m); > + } > + } > + printf "};\n" > print "#endif" > }