[v2,4/4] elf: Make glibc.rtld.enable_secure ignore alias environment variables
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
Commit Message
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.
---
elf/dl-tunables.c | 36 ++++++---
elf/tst-tunables-enable_secure.c | 133 +++++++++++++++++++++++++++----
scripts/gen-tunables.awk | 16 +++-
3 files changed, 161 insertions(+), 24 deletions(-)
Comments
On 2024-05-02 12:35, 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.
> ---
> elf/dl-tunables.c | 36 ++++++---
> elf/tst-tunables-enable_secure.c | 133 +++++++++++++++++++++++++++----
> scripts/gen-tunables.awk | 16 +++-
> 3 files changed, 161 insertions(+), 24 deletions(-)
>
> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 63cf8c7ab5..e87d0628b2 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;
You don't skip over initialized ones here because you're going to have
to check for that later anyway, in case GLIBC_TUNABLES came in after any
of the aliases. OK.
>
> - 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. */
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..a0da0fd179 100644
> --- a/elf/tst-tunables-enable_secure.c
> +++ b/elf/tst-tunables-enable_secure.c
> @@ -17,6 +17,7 @@
> <https://www.gnu.org/licenses/>. */
>
> #include <array_length.h>
> +#define TUNABLES_INTERNAL 1
OK, I noticed this in patch 1 but it didn't strike me then; this macro
will result in a new array tunable_list being initialized in the binary.
The tunable references themselves should go via __tunable_get_val and
hence refer to the structure in ld.so, so this array only serves as a
copy to get the total size. I suppose it's not much for a test case,
but do you think it's worth adding a comment here, and also in
tst-tunables.c? That way it could be easier to weed out any unforeseen
problems with these tests in future.
> #include <dl-tunables.h>
> #include <getopt.h>
> #include <intprops.h>
> @@ -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",
Same question as in v1, should we issue a warning for enable_secure=0?
We really should never be able to set it to anything other than 1.
Which also means that a bunch of these enable_secure=0 tests are
probably redundant.
> + 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 +194,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__ = {"
Why is this needed? Doesn't array_length ensures at least one "use"?
> 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"
> }
On 03/05/24 12:30, Siddhesh Poyarekar wrote:
> On 2024-05-02 12:35, 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.
>> ---
>> elf/dl-tunables.c | 36 ++++++---
>> elf/tst-tunables-enable_secure.c | 133 +++++++++++++++++++++++++++----
>> scripts/gen-tunables.awk | 16 +++-
>> 3 files changed, 161 insertions(+), 24 deletions(-)
>>
>> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
>> index 63cf8c7ab5..e87d0628b2 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;
>
> You don't skip over initialized ones here because you're going to have to check for that later anyway, in case GLIBC_TUNABLES came in after any of the aliases. OK.
>
>> - 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. */
>
> either been set or already initialized?
Ack.
>
>> + 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..a0da0fd179 100644
>> --- a/elf/tst-tunables-enable_secure.c
>> +++ b/elf/tst-tunables-enable_secure.c
>> @@ -17,6 +17,7 @@
>> <https://www.gnu.org/licenses/>. */
>> #include <array_length.h>
>> +#define TUNABLES_INTERNAL 1
>
> OK, I noticed this in patch 1 but it didn't strike me then; this macro will result in a new array tunable_list being initialized in the binary. The tunable references themselves should go via __tunable_get_val and hence refer to the structure in ld.so, so this array only serves as a copy to get the total size. I suppose it's not much for a test case, but do you think it's worth adding a comment here, and also in tst-tunables.c? That way it could be easier to weed out any unforeseen problems with these tests in future.
Ok, I will add a comment why this is required.
>
>> #include <dl-tunables.h>
>> #include <getopt.h>
>> #include <intprops.h>
>> @@ -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",
>
> Same question as in v1, should we issue a warning for enable_secure=0? We really should never be able to set it to anything other than 1. Which also means that a bunch of these enable_secure=0 tests are probably redundant.
I am not sure if adding another knob for an specific tunable, specially one that
is only aimed for debug, would yield much gain here. The enable_secure=0 is just
the default value, similar to setting any other tunable to its default value.
>
>> + 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 +194,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__ = {"
>
> Why is this needed? Doesn't array_length ensures at least one "use"?
The array_length is used for only for tunable_env_alias_list on
elf/tst-tunables-enable_secure.c.
>
>> 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"
>> }
@@ -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. */
+ 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
@@ -17,6 +17,7 @@
<https://www.gnu.org/licenses/>. */
#include <array_length.h>
+#define TUNABLES_INTERNAL 1
#include <dl-tunables.h>
#include <getopt.h>
#include <intprops.h>
@@ -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;
}
@@ -106,14 +194,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);
@@ -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"
}