| Message ID | 20240430192739.1032549-5-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | More tunable fixes | expand |
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 @@ > <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; > } > > @@ -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 <dl-procinfo.h>\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" > }
On 01/05/24 14:40, Siddhesh Poyarekar wrote: > 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. I though about it and I decided to change tunables sorting because this is add extra overhead to a routine that is issue for every new process. Updating the system libc was never atomically due the current constraints (maybe in the future if/when we move all libc to ld.so), so I am not really convinced that we should make the tunables ordering list an ABI constraint. > > 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 @@ >> <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; >> } >> @@ -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 <dl-procinfo.h>\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" >> }
On 2024-05-01 14:00, Adhemerval Zanella Netto wrote: > > > On 01/05/24 14:40, Siddhesh Poyarekar wrote: >> 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. > > I though about it and I decided to change tunables sorting because this is > add extra overhead to a routine that is issue for every new process. Updating > the system libc was never atomically due the current constraints (maybe in > the future if/when we move all libc to ld.so), so I am not really convinced > that we should make the tunables ordering list an ABI constraint. I don't have a really strong opinion either way, but it's something we've run into often enough in Fedora for it to be pointed out as a potential problem. The right solution for this problem is in fact to make the TUNABLE_GET API independent of the tunable ID so that the tunables can move around as needed, but I don't think this patch needs to block on that. Florian, do you have an opinion on this? > >> >> 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 @@ >>> <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. */ s/Tunable/Tunables/ >>> + "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", This gives the false impression that enable_secure can be disabled. The only valid value to set this tunable is 1; 0 should be ignored. In fact, now I wonder if setting enable_secure to 0 should emit a warning, like for other invalid tunables. That's independent of this change of course. >>> + /* 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 <dl-procinfo.h>\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" >>> } >
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 @@ <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; } @@ -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 <dl-procinfo.h>\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" }