stdlib: Re-implement free (environ) compatibility kludge for setenv

For the originally failing application (userhelper from usermode),
it is not actually necessary to call realloc on the environ
pointer.  Yes, there will be a memory leak because the application
assigns a heap-allocated pointer to environ that it never frees,
but this leak was always there: the old realloc-based setenv had
a hidden internal variable, last_environ, that was used in a similar
way to __environ_array_list.  The application is not impacted by
the leak anyway because the relevant operations do not happen in
a loop.

The change here just uses a separte heap allocation and points
environ to that.  This means that if an application calls
free (environ) and restores the environ pointer to the value
at process start, and does not modify the environment further,
nothing bad happens.

This change should not invalidate any previous testing that went into
the original getenv thread safety change, commit 7a61e7f557a97ab597d6
("stdlib: Make getenv thread-safe in more cases").

The new test cases are modeled in part on the env -i use case from
bug 32588 (with !DO_MALLOC && !DO_EARLY_SETENV), and the previous
stdlib/tst-setenv-malloc test.  The DO_MALLOC && !DO_EARLY_SETENV
case in the new test should approximate what userhelper from the
usermode package does.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

1 files changed, 12 insertions, 3 deletions

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.