[v2,04/19] elf: Add all malloc tunable to unsecvars
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
Commit Message
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.
---
elf/Makefile | 8 +--
elf/tst-env-setuid-static.c | 1 +
elf/tst-env-setuid.c | 128 +++++++++++++++++++++---------------
sysdeps/generic/unsecvars.h | 7 ++
4 files changed, 86 insertions(+), 58 deletions(-)
create mode 100644 elf/tst-env-setuid-static.c
Comments
On 2023-10-17 09:05, Adhemerval Zanella wrote:
> 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>
Likewise, I'll leave this for someone else since I am a co-author.
Thanks,
Sid
>
> Checked on x86_64-linux-gnu.
> ---
> elf/Makefile | 8 +--
> elf/tst-env-setuid-static.c | 1 +
> elf/tst-env-setuid.c | 128 +++++++++++++++++++++---------------
> sysdeps/generic/unsecvars.h | 7 ++
> 4 files changed, 86 insertions(+), 58 deletions(-)
> create mode 100644 elf/tst-env-setuid-static.c
>
> diff --git a/elf/Makefile b/elf/Makefile
> index c824f7dab7..f1cd6e13fa 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 \
> @@ -305,6 +305,7 @@ tests := \
> tst-array5 \
> tst-auxv \
> tst-dl-hash \
> + tst-env-setuid \
> tst-leaks1 \
> tst-stringtable \
> tst-tls9 \
> @@ -2467,9 +2468,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
> @@ -3021,3 +3019,5 @@ LDFLAGS-tst-dlclose-lazy-mod1.so = -Wl,-z,lazy,--no-as-needed
> $(objpfx)tst-dlclose-lazy-mod1.so: $(objpfx)tst-dlclose-lazy-mod2.so
> $(objpfx)tst-dlclose-lazy.out: \
> $(objpfx)tst-dlclose-lazy-mod1.so $(objpfx)tst-dlclose-lazy-mod2.so
> +
> +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" \
On 2023-10-18 09:04, Siddhesh Poyarekar wrote:
>
>
> On 2023-10-17 09:05, Adhemerval Zanella wrote:
>> 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>
>
> Likewise, I'll leave this for someone else since I am a co-author.
Carlos, can you please review this?
Thanks,
Sid
>
> Thanks,
> Sid
>
>>
>> Checked on x86_64-linux-gnu.
>> ---
>> elf/Makefile | 8 +--
>> elf/tst-env-setuid-static.c | 1 +
>> elf/tst-env-setuid.c | 128 +++++++++++++++++++++---------------
>> sysdeps/generic/unsecvars.h | 7 ++
>> 4 files changed, 86 insertions(+), 58 deletions(-)
>> create mode 100644 elf/tst-env-setuid-static.c
>>
>> diff --git a/elf/Makefile b/elf/Makefile
>> index c824f7dab7..f1cd6e13fa 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 \
>> @@ -305,6 +305,7 @@ tests := \
>> tst-array5 \
>> tst-auxv \
>> tst-dl-hash \
>> + tst-env-setuid \
>> tst-leaks1 \
>> tst-stringtable \
>> tst-tls9 \
>> @@ -2467,9 +2468,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
>> @@ -3021,3 +3019,5 @@ LDFLAGS-tst-dlclose-lazy-mod1.so =
>> -Wl,-z,lazy,--no-as-needed
>> $(objpfx)tst-dlclose-lazy-mod1.so: $(objpfx)tst-dlclose-lazy-mod2.so
>> $(objpfx)tst-dlclose-lazy.out: \
>> $(objpfx)tst-dlclose-lazy-mod1.so $(objpfx)tst-dlclose-lazy-mod2.so
>> +
>> +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" \
>
LGTM.
Reviewed-by: DJ Delorie <dj@redhat.com>
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> diff --git a/elf/Makefile b/elf/Makefile
> index c824f7dab7..f1cd6e13fa 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 \
> @@ -305,6 +305,7 @@ tests := \
> tst-array5 \
> tst-auxv \
> tst-dl-hash \
> + tst-env-setuid \
> tst-leaks1 \
> tst-stringtable \
> tst-tls9 \
Ok.
> @@ -2467,9 +2468,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
> -
Ok.
> @@ -3021,3 +3019,5 @@ LDFLAGS-tst-dlclose-lazy-mod1.so = -Wl,-z,lazy,--no-as-needed
> $(objpfx)tst-dlclose-lazy-mod1.so: $(objpfx)tst-dlclose-lazy-mod2.so
> $(objpfx)tst-dlclose-lazy.out: \
> $(objpfx)tst-dlclose-lazy-mod1.so $(objpfx)tst-dlclose-lazy-mod2.so
> +
> +tst-env-setuid-ARGS = -- $(host-test-program-cmd)
Ok.
> 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"
Ok.
> 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. */
Ok.
> -#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>
Ok.
> #include <support/check.h>
> @@ -36,61 +32,72 @@
>
> static char SETGID_CHILD[] = "setgid-child";
>
> +#define FILTERED_VALUE "some-filtered-value"
> +#define UNFILTERED_VALUE "some-unfiltered-value"
Ok.
> +struct envvar_t
> +{
> + const char *env;
> + const char *value;
> +};
Ok.
> -#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;
> - }
>
> - if (getenv ("LD_HWCAP_MASK") != NULL)
> - {
> - printf ("LD_HWCAP_MASK still set\n");
> - return 1;
> - }
> - return 0;
> -}
> -#endif
Ok.
> +/* 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 },
> +};
Ok.
> -#ifndef test_parent
> static int
> -test_parent (void)
> +test_child (void)
Ok.
> {
> - if (getenv ("MALLOC_CHECK_") == NULL)
> - {
> - printf ("MALLOC_CHECK_ lost\n");
> - return 1;
> - }
Ok.
> + 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;
> }
Ok. Diff is not our friend here ;-)
Checking for vars which should be NOT set.
> - 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);
> }
Ok. Checking for vars which should be set.
> - return 0;
> + return ret;
> }
> -#endif
Ok.
> 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");
> +
Does not test for argc < 2 but this is internal so OK.
> @@ -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);
> }
Ok.
> 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);
Ok.
> 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);
Ok.
> + if (WEXITSTATUS (status) != 42)
> + {
> + printf (" child failed with status %d\n",
> + WEXITSTATUS (status));
> + support_record_failure ();
> + }
Ok.
> return 0;
> }
Ok.
> 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" \
Ok.
@@ -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 \
@@ -305,6 +305,7 @@ tests := \
tst-array5 \
tst-auxv \
tst-dl-hash \
+ tst-env-setuid \
tst-leaks1 \
tst-stringtable \
tst-tls9 \
@@ -2467,9 +2468,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
@@ -3021,3 +3019,5 @@ LDFLAGS-tst-dlclose-lazy-mod1.so = -Wl,-z,lazy,--no-as-needed
$(objpfx)tst-dlclose-lazy-mod1.so: $(objpfx)tst-dlclose-lazy-mod2.so
$(objpfx)tst-dlclose-lazy.out: \
$(objpfx)tst-dlclose-lazy-mod1.so $(objpfx)tst-dlclose-lazy-mod2.so
+
+tst-env-setuid-ARGS = -- $(host-test-program-cmd)
new file mode 100644
@@ -0,0 +1 @@
+#include "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;
}
@@ -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" \