diff options
author | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2023-11-06 17:25:37 -0300 |
---|---|---|
committer | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2023-11-21 16:15:42 -0300 |
commit | 11f7e3dd8fed66e0b8740af440cd3151e55a466f (patch) | |
tree | 69169ddfeb2a8d757a42de6e00707dc4ff247f7f | |
parent | 9c96c87d60eafa4d78406e606e92b42bd4b570ad (diff) | |
download | glibc-11f7e3dd8fed66e0b8740af440cd3151e55a466f.tar.gz glibc-11f7e3dd8fed66e0b8740af440cd3151e55a466f.tar.xz glibc-11f7e3dd8fed66e0b8740af440cd3151e55a466f.zip |
elf: Add all malloc tunable to unsecvars
Some environment variables allow alteration of allocator behavior across setuid boundaries, where a setuid program may ignore the tunable, but its non-setuid child can read it and adjust the memory allocator behavior accordingly. Most library behavior tunings is limited to the current process and does not bleed in scope; so it is unclear how pratical this misfeature is. If behavior change across privilege boundaries is desirable, it would be better done with a wrapper program around the non-setuid child that sets these envvars, instead of using the setuid process as the messenger. The patch as fixes tst-env-setuid, where it fail if any unsecvars is set. It also adds a dynamic test, although it requires --enable-hardcoded-path-in-tests so kernel correctly sets the setuid bit (using the loader command directly would require to set the setuid bit on the loader itself, which is not a usual deployment). Co-authored-by: Siddhesh Poyarekar <siddhesh@sourceware.org> Checked on x86_64-linux-gnu. Reviewed-by: DJ Delorie <dj@redhat.com>
-rw-r--r-- | elf/Makefile | 8 | ||||
-rw-r--r-- | elf/tst-env-setuid-static.c | 1 | ||||
-rw-r--r-- | elf/tst-env-setuid.c | 128 | ||||
-rw-r--r-- | sysdeps/generic/unsecvars.h | 7 |
4 files changed, 86 insertions, 58 deletions
diff --git a/elf/Makefile b/elf/Makefile index 761f1d0af3..1af8ca4f84 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -262,7 +262,7 @@ tests-static-normal := \ tst-array5-static \ tst-dl-iter-static \ tst-dst-static \ - tst-env-setuid \ + tst-env-setuid-static \ tst-getauxval-static \ tst-linkall-static \ tst-single_threaded-pthread-static \ @@ -306,6 +306,7 @@ tests := \ tst-auxv \ tst-decorate-maps \ tst-dl-hash \ + tst-env-setuid \ tst-leaks1 \ tst-stringtable \ tst-tls9 \ @@ -2433,9 +2434,6 @@ $(objpfx)tst-nodelete-dlclose: $(objpfx)tst-nodelete-dlclose-dso.so $(objpfx)tst-nodelete-dlclose.out: $(objpfx)tst-nodelete-dlclose-dso.so \ $(objpfx)tst-nodelete-dlclose-plugin.so -tst-env-setuid-ENV = MALLOC_CHECK_=2 MALLOC_MMAP_THRESHOLD_=4096 \ - LD_HWCAP_MASK=0x1 - $(objpfx)tst-debug1.out: $(objpfx)tst-debug1mod1.so $(objpfx)tst-debug1mod1.so: $(objpfx)testobj1.so @@ -3002,3 +3000,5 @@ $(objpfx)tst-non-directory-path.out: tst-non-directory-path.sh \ '$(test-wrapper-env)' '$(run_program_env)' \ '$(rpath-link)' $(objpfx) > $@; \ $(evaluate-test) + +tst-env-setuid-ARGS = -- $(host-test-program-cmd) diff --git a/elf/tst-env-setuid-static.c b/elf/tst-env-setuid-static.c new file mode 100644 index 0000000000..0d88ae88b9 --- /dev/null +++ b/elf/tst-env-setuid-static.c @@ -0,0 +1 @@ +#include "tst-env-setuid.c" diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c index 032ab44be2..ba295a6a14 100644 --- a/elf/tst-env-setuid.c +++ b/elf/tst-env-setuid.c @@ -15,18 +15,14 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -/* Verify that tunables correctly filter out unsafe environment variables like - MALLOC_CHECK_ and MALLOC_MMAP_THRESHOLD_ but also retain - MALLOC_MMAP_THRESHOLD_ in an unprivileged child. */ +/* Verify that correctly filter out unsafe environment variables defined + in unsecvars.h. */ -#include <errno.h> -#include <fcntl.h> -#include <stdlib.h> -#include <stdint.h> +#include <array_length.h> +#include <gnu/lib-names.h> #include <stdio.h> +#include <stdlib.h> #include <string.h> -#include <sys/stat.h> -#include <sys/wait.h> #include <unistd.h> #include <support/check.h> @@ -36,61 +32,72 @@ static char SETGID_CHILD[] = "setgid-child"; -#ifndef test_child -static int -test_child (void) -{ - if (getenv ("MALLOC_CHECK_") != NULL) - { - printf ("MALLOC_CHECK_ is still set\n"); - return 1; - } - - if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL) - { - printf ("MALLOC_MMAP_THRESHOLD_ lost\n"); - return 1; - } +#define FILTERED_VALUE "some-filtered-value" +#define UNFILTERED_VALUE "some-unfiltered-value" - if (getenv ("LD_HWCAP_MASK") != NULL) - { - printf ("LD_HWCAP_MASK still set\n"); - return 1; - } +struct envvar_t +{ + const char *env; + const char *value; +}; - return 0; -} -#endif +/* That is not an extensible list of all filtered out environment + variables. */ +static const struct envvar_t filtered_envvars[] = +{ + { "GLIBC_TUNABLES", FILTERED_VALUE }, + { "LD_AUDIT", FILTERED_VALUE }, + { "LD_HWCAP_MASK", FILTERED_VALUE }, + { "LD_LIBRARY_PATH", FILTERED_VALUE }, + { "LD_PRELOAD", FILTERED_VALUE }, + { "LD_PROFILE", FILTERED_VALUE }, + { "MALLOC_ARENA_MAX", FILTERED_VALUE }, + { "MALLOC_PERTURB_", FILTERED_VALUE }, + { "MALLOC_TRACE", FILTERED_VALUE }, + { "MALLOC_TRIM_THRESHOLD_", FILTERED_VALUE }, + { "RES_OPTIONS", FILTERED_VALUE }, +}; + +static const struct envvar_t unfiltered_envvars[] = +{ + { "LD_BIND_NOW", "0" }, + { "LD_BIND_NOT", "1" }, + /* Non longer supported option. */ + { "LD_ASSUME_KERNEL", UNFILTERED_VALUE }, +}; -#ifndef test_parent static int -test_parent (void) +test_child (void) { - if (getenv ("MALLOC_CHECK_") == NULL) - { - printf ("MALLOC_CHECK_ lost\n"); - return 1; - } + int ret = 0; - if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL) + for (const struct envvar_t *e = filtered_envvars; + e != array_end (filtered_envvars); + e++) { - printf ("MALLOC_MMAP_THRESHOLD_ lost\n"); - return 1; + const char *env = getenv (e->env); + ret |= env != NULL; } - if (getenv ("LD_HWCAP_MASK") == NULL) + for (const struct envvar_t *e = unfiltered_envvars; + e != array_end (unfiltered_envvars); + e++) { - printf ("LD_HWCAP_MASK lost\n"); - return 1; + const char *env = getenv (e->env); + ret |= !(env != NULL && strcmp (env, e->value) == 0); } - return 0; + return ret; } -#endif static int do_test (int argc, char **argv) { + /* For dynamic loader, the test requires --enable-hardcoded-path-in-tests so + the kernel sets the AT_SECURE on process initialization. */ + if (argc >= 2 && strstr (argv[1], LD_SO) != 0) + FAIL_UNSUPPORTED ("dynamic test requires --enable-hardcoded-path-in-tests"); + /* Setgid child process. */ if (argc == 2 && strcmp (argv[1], SETGID_CHILD) == 0) { @@ -104,20 +111,33 @@ do_test (int argc, char **argv) if (ret != 0) exit (1); - exit (EXIT_SUCCESS); + /* Special return code to make sure that the child executed all the way + through. */ + exit (42); } else { - if (test_parent () != 0) - exit (1); + for (const struct envvar_t *e = filtered_envvars; + e != array_end (filtered_envvars); + e++) + setenv (e->env, e->value, 1); + + for (const struct envvar_t *e = unfiltered_envvars; + e != array_end (unfiltered_envvars); + e++) + setenv (e->env, e->value, 1); int status = support_capture_subprogram_self_sgid (SETGID_CHILD); if (WEXITSTATUS (status) == EXIT_UNSUPPORTED) - return EXIT_UNSUPPORTED; - - if (!WIFEXITED (status)) - FAIL_EXIT1 ("Unexpected exit status %d from child process\n", status); + exit (EXIT_UNSUPPORTED); + + if (WEXITSTATUS (status) != 42) + { + printf (" child failed with status %d\n", + WEXITSTATUS (status)); + support_record_failure (); + } return 0; } diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h index 81397fb90b..f7ebed60e5 100644 --- a/sysdeps/generic/unsecvars.h +++ b/sysdeps/generic/unsecvars.h @@ -18,7 +18,14 @@ "LD_SHOW_AUXV\0" \ "LOCALDOMAIN\0" \ "LOCPATH\0" \ + "MALLOC_ARENA_MAX\0" \ + "MALLOC_ARENA_TEST\0" \ + "MALLOC_MMAP_MAX_\0" \ + "MALLOC_MMAP_THRESHOLD_\0" \ + "MALLOC_PERTURB_\0" \ + "MALLOC_TOP_PAD_\0" \ "MALLOC_TRACE\0" \ + "MALLOC_TRIM_THRESHOLD_\0" \ "NIS_PATH\0" \ "NLSPATH\0" \ "RESOLV_HOST_CONF\0" \ |