diff --git a/stdlib/Makefile b/stdlib/Makefile index a5fbc1a27e..c8b96b329e 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -277,6 +277,10 @@ tests := \ tst-concurrent-quick_exit \ tst-cxa_atexit \ tst-environ \ + tst-environ-change-1 \ + tst-environ-change-2 \ + tst-environ-change-3 \ + tst-environ-change-4 \ tst-getenv-signal \ tst-getenv-thread \ tst-getenv-unsetenv \ diff --git a/stdlib/setenv.c b/stdlib/setenv.c index 2a2eec9c98..0ef5dde373 100644 --- a/stdlib/setenv.c +++ b/stdlib/setenv.c @@ -118,24 +118,21 @@ __environ_new_array (size_t required_size) else new_size = __environ_array_list->allocated * 2; - size_t new_size_in_bytes; - if (__builtin_mul_overflow (new_size, sizeof (char *), - &new_size_in_bytes) - || __builtin_add_overflow (new_size_in_bytes, - offsetof (struct environ_array, - array), - &new_size_in_bytes)) + /* Zero-initialize everything, so that getenv can only + observe valid or null pointers. */ + char **new_array = calloc (new_size, sizeof (*new_array)); + if (new_array == NULL) + return NULL; + + struct environ_array *target_array = malloc (sizeof (*target_array)); + if (target_array == NULL) { - __set_errno (ENOMEM); + free (new_array); return NULL; } - /* Zero-initialize everything, so that getenv can only - observe valid or null pointers. */ - struct environ_array *target_array = calloc (1, new_size_in_bytes); - if (target_array == NULL) - return NULL; target_array->allocated = new_size; + target_array->array = new_array; assert (new_size >= target_array->allocated); /* Put it onto the list. */ @@ -236,7 +233,7 @@ __add_to_environ (const char *name, const char *value, const char *combined, ep[1] = NULL; /* And __environ should be repointed to our array. */ - result_environ = &target_array->array[0]; + result_environ = target_array->array; } } @@ -403,6 +400,7 @@ __libc_setenv_freemem (void) /* Clear all backing arrays. */ while (__environ_array_list != NULL) { + free (__environ_array_list->array); void *ptr = __environ_array_list; __environ_array_list = __environ_array_list->next; free (ptr); diff --git a/stdlib/setenv.h b/stdlib/setenv.h index e4433f5f84..7cbf9f2059 100644 --- a/stdlib/setenv.h +++ b/stdlib/setenv.h @@ -29,9 +29,18 @@ of environment values used before. */ struct environ_array { - struct environ_array *next; /* Previously used environment array. */ + /* The actual environment array. Use a separate allocation (and not + a flexible array member) so that calls like free (environ) that + have been encountered in some applications do not crash + immediately. With such a call, if the application restores the + original environ pointer at process start and does not modify the + environment again, a use-after-free situation only occurs during + __libc_freeres, which is only called during memory debugging. + With subsequent setenv calls, there is still heap corruption, but + that happened with the old realloc-based implementation, too. */ + char **array; size_t allocated; /* Number of allocated array elments. */ - char *array[]; /* The actual environment array. */ + struct environ_array *next; /* Previously used environment array. */ }; /* After initialization, and until the user resets environ (perhaps by @@ -44,7 +53,7 @@ static inline bool __environ_is_from_array_list (char **ep) { struct environ_array *eal = atomic_load_relaxed (&__environ_array_list); - return eal != NULL && &eal->array[0] == ep; + return eal != NULL && eal->array == ep; } /* Counter for detecting concurrent modification in unsetenv. diff --git a/stdlib/tst-environ-change-1.c b/stdlib/tst-environ-change-1.c new file mode 100644 index 0000000000..4241ad4c63 --- /dev/null +++ b/stdlib/tst-environ-change-1.c @@ -0,0 +1,3 @@ +#define DO_EARLY_SETENV 0 +#define DO_MALLOC 0 +#include "tst-environ-change-skeleton.c" diff --git a/stdlib/tst-environ-change-2.c b/stdlib/tst-environ-change-2.c new file mode 100644 index 0000000000..b20be12490 --- /dev/null +++ b/stdlib/tst-environ-change-2.c @@ -0,0 +1,3 @@ +#define DO_EARLY_SETENV 0 +#define DO_MALLOC 1 +#include "tst-environ-change-skeleton.c" diff --git a/stdlib/tst-environ-change-3.c b/stdlib/tst-environ-change-3.c new file mode 100644 index 0000000000..e77996a6cb --- /dev/null +++ b/stdlib/tst-environ-change-3.c @@ -0,0 +1,3 @@ +#define DO_EARLY_SETENV 1 +#define DO_MALLOC 0 +#include "tst-environ-change-skeleton.c" diff --git a/stdlib/tst-environ-change-4.c b/stdlib/tst-environ-change-4.c new file mode 100644 index 0000000000..633ef7bda8 --- /dev/null +++ b/stdlib/tst-environ-change-4.c @@ -0,0 +1,3 @@ +#define DO_EARLY_SETENV 1 +#define DO_MALLOC 1 +#include "tst-environ-change-skeleton.c" diff --git a/stdlib/tst-environ-change-skeleton.c b/stdlib/tst-environ-change-skeleton.c new file mode 100644 index 0000000000..c9b0284436 --- /dev/null +++ b/stdlib/tst-environ-change-skeleton.c @@ -0,0 +1,118 @@ +/* Test deallocation of the environ pointer. + Copyright (C) 2025 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* This test is not in the scope for POSIX or any other standard, but + some applications assume that environ is a heap-allocated pointer + after a call to setenv on an empty environment. They also try to + save and restore environ in an attempt to undo a temporary + modification of the environment array, but this does not work if + setenv was called before. + + Before including this file, these macros need to be defined + to 0 or 1: + + DO_EARLY_SETENV If 1, perform a setenv call before changing environ. + DO_MALLOC If 1, use a heap pointer for the empty environment. + + Note that this test will produce errors under valgrind and other + memory tracers that call __libc_freeres because free (environ) + deallocates a pointer still used internally. */ + +#include +#include +#include +#include + +static void +check_rewritten (void) +{ + TEST_COMPARE_STRING (environ[0], "tst_environ_change_a=1"); + TEST_COMPARE_STRING (environ[1], "tst_environ_change_b=2"); + TEST_COMPARE_STRING (environ[2], NULL); + TEST_COMPARE_STRING (getenv ("tst_environ_change_a"), "1"); + TEST_COMPARE_STRING (getenv ("tst_environ_change_b"), "2"); + TEST_COMPARE_STRING (getenv ("tst_environ_change_early"), NULL); + TEST_COMPARE_STRING (getenv ("PATH"), NULL); +} + +static int +do_test (void) +{ + TEST_COMPARE_STRING (getenv ("tst_environ_change_a"), NULL); + TEST_COMPARE_STRING (getenv ("tst_environ_change_b"), NULL); + TEST_COMPARE_STRING (getenv ("tst_environ_change_early_setenv"), NULL); +#if DO_EARLY_SETENV + TEST_COMPARE (setenv ("tst_environ_change_early_setenv", "1", 1), 0); +#else + /* Must come back after environ reset. */ + char *original_path = xstrdup (getenv ("PATH")); +#endif + + char **save_environ = environ; +#if DO_MALLOC + environ = xmalloc (sizeof (*environ)); +#else + char *environ_array[1]; + environ = environ_array; +#endif + *environ = NULL; + TEST_COMPARE (setenv ("tst_environ_change_a", "1", 1), 0); + TEST_COMPARE (setenv ("tst_environ_change_b", "2", 1), 0); +#if !DO_EARLY_SETENV + /* Early setenv results in reuse of the heap-allocated environ array + that does not change as more pointers are added to it. */ + TEST_VERIFY (environ != save_environ); +#endif + check_rewritten (); + + bool check_environ = true; +#if DO_MALLOC + /* Disable further checks if the free call clobbers the environ + contents. Whether that is the case depends on the internal + setenv allocation policy and the heap layout. */ + check_environ = environ != save_environ; + /* Invalid: Causes internal use-after-free condition. Yet this has + to be supported for compatibility with some applications. */ + free (environ); +#endif + + environ = save_environ; + +#if DO_EARLY_SETENV + /* With an early setenv, the internal environ array was overwritten. + Historically, this triggered a use-after-free problem because of + the use of realloc internally in setenv, but it may appear as if + the original environment had been restored. In the current code, + we can only support this if the free (environ) above call did not + clobber the array, otherwise getenv will see invalid pointers. + Due to the use-after-free, invalid pointers could be seen with + the old implementation as well, but the triggering conditions + were different. */ + if (check_environ) + check_rewritten (); +#else + TEST_VERIFY (check_environ); + TEST_COMPARE_STRING (getenv ("PATH"), original_path); + TEST_COMPARE_STRING (getenv ("tst_environ_change_a"), NULL); + TEST_COMPARE_STRING (getenv ("tst_environ_change_b"), NULL); +#endif + + return 0; +} + +#include