rpms/glibc
5
0
Fork 2
Code Issues Pull Requests Releases Activity

setenv: Rework free(environ) compatibility support (#2341908)

Browse Source 
Resolves: RHEL-75809
This commit is contained in:
Florian Weimer 2025-01-24 23:13:53 +01:00
parent b76546a663
commit 2fac143e74
3 changed files with 537 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

232
glibc-RHEL-75809-2.patch Normal file
View File

@ -0,0 +1,232 @@
commit 36fcdfbbc5463e55581fec67141df3493fb81f7e
Author: Florian Weimer <fweimer@redhat.com>
Date: Fri Jan 24 08:04:23 2025 +0100
Revert "stdlib: Support malloc-managed environ arrays for compatibility"
This reverts commit b62759db04b8ed7f829c06f1d7c3b8fb70616493.
Reason for revert: Incompatible with “env -i” and coreutils (bug 32588).
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
diff --git a/csu/init-first.c b/csu/init-first.c
index 77b5b4941beb3a73..a2cb456ccf9ac5e6 100644
--- a/csu/init-first.c
+++ b/csu/init-first.c
@@ -61,7 +61,6 @@ _init_first (int argc, char **argv, char **envp)
__libc_argc = argc;
__libc_argv = argv;
__environ = envp;
- __environ_startup = envp;
#ifndef SHARED
/* First the initialization which normally would be done by the
diff --git a/csu/libc-start.c b/csu/libc-start.c
index 260027c2396e1f52..d784de0f0bdd70c8 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -244,7 +244,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
char **ev = &argv[argc + 1];
__environ = ev;
- __environ_startup = ev;
/* Store the lowest stack address. This is done in ld.so if this is
the code for the DSO. */
diff --git a/include/unistd.h b/include/unistd.h
index ada957f9d04d272a..e241603b8131a9e9 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -203,9 +203,6 @@ libc_hidden_proto (__tcsetpgrp)
extern int __libc_enable_secure attribute_relro;
rtld_hidden_proto (__libc_enable_secure)
-/* Original value of __environ. Initialized by _init_first (dynamic)
- or __libc_start_main (static). */
-extern char **__environ_startup attribute_hidden;
/* Various internal function. */
extern void __libc_check_standard_fds (void) attribute_hidden;
diff --git a/posix/environ.c b/posix/environ.c
index 2430b47d8eee148c..a0ed0d80eab207f8 100644
--- a/posix/environ.c
+++ b/posix/environ.c
@@ -10,5 +10,3 @@ weak_alias (__environ, environ)
/* The SVR4 ABI says `_environ' will be the name to use
in case the user overrides the weak alias `environ'. */
weak_alias (__environ, _environ)
-
-char **__environ_startup;
diff --git a/stdlib/Makefile b/stdlib/Makefile
index 217600ba60e3c7d4..ff1418f5bb2ea5c9 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -312,7 +312,6 @@ tests := \
tst-setcontext9 \
tst-setcontext10 \
tst-setcontext11 \
- tst-setenv-malloc \
tst-stdbit-Wconversion \
tst-stdbit-builtins \
tst-stdc_bit_ceil \
diff --git a/stdlib/setenv.c b/stdlib/setenv.c
index 79982aa12ac20078..d12401ca77cee5a7 100644
--- a/stdlib/setenv.c
+++ b/stdlib/setenv.c
@@ -191,52 +191,52 @@ __add_to_environ (const char *name, const char *value, const char *combined,
ep[1] = NULL;
else
{
- /* We cannot use __environ as is and need a larger allocation. */
-
- if (start_environ == __environ_startup
- || __environ_is_from_array_list (start_environ))
- {
- /* Allocate a new array, managed in the list. */
- struct environ_array *target_array
- = __environ_new_array (required_size);
- if (target_array == NULL)
- {
- UNLOCK;
- return -1;
- }
- result_environ = &target_array->array[0];
-
- /* Copy over the __environ array contents. This code
- handles the case start_environ == ep == NULL, too. */
- size_t i;
- for (i = 0; start_environ + i < ep; ++i)
- /* Regular store because unless there has been direct
- manipulation of the environment, target_array is still
- a private copy. */
- result_environ[i] = atomic_load_relaxed (start_environ + i);
- }
+ /* We cannot use __environ as is and need to copy over the
+ __environ contents into an array managed via
+ __environ_array_list. */
+
+ struct environ_array *target_array;
+ if (__environ_array_list != NULL
+ && required_size <= __environ_array_list->allocated)
+ /* Existing array has enough room. Contents is copied below. */
+ target_array = __environ_array_list;
else
{
- /* Otherwise the application installed its own pointer.
- Historically, this pointer was managed using realloc.
- Continue doing so. This disables multi-threading
- support. */
- result_environ = __libc_reallocarray (start_environ,
- required_size,
- sizeof (*result_environ));
- if (result_environ == NULL)
+ /* Allocate a new array. */
+ target_array = __environ_new_array (required_size);
+ if (target_array == NULL)
{
UNLOCK;
return -1;
}
}
+ /* Copy over the __environ array contents. This forward
+ copy slides backwards part of the array if __environ
+ points into target_array->array. This happens if an
+ application makes an assignment like:
+
+ environ = &environ[1];
+
+ The forward copy avoids clobbering values that still
+ needing copying. This code handles the case
+ start_environ == ep == NULL, too. */
+ size_t i;
+ for (i = 0; start_environ + i < ep; ++i)
+ /* Regular store because unless there has been direct
+ manipulation of the environment, target_array is still
+ a private copy. */
+ target_array->array[i] = atomic_load_relaxed (start_environ + i);
+
/* This is the new place where we should add the element. */
- ep = result_environ + (required_size - 2);
+ ep = target_array->array + i;
/* Add the null terminator in case there was a pointer there
previously. */
ep[1] = NULL;
+
+ /* And __environ should be repointed to our array. */
+ result_environ = &target_array->array[0];
}
}
diff --git a/stdlib/tst-setenv-malloc.c b/stdlib/tst-setenv-malloc.c
deleted file mode 100644
index 18a9d36842e67aa5..0000000000000000
--- a/stdlib/tst-setenv-malloc.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/* Test using setenv with a malloc-allocated environ variable.
- 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
- <https://www.gnu.org/licenses/>. */
-
-/* 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. */
-
-#include <stdlib.h>
-#include <unistd.h>
-#include <support/check.h>
-#include <support/support.h>
-
-static const char *original_path;
-static char **save_environ;
-
-static void
-rewrite_environ (void)
-{
- save_environ = environ;
- environ = xmalloc (sizeof (*environ));
- *environ = NULL;
- TEST_COMPARE (setenv ("A", "1", 1), 0);
- TEST_COMPARE (setenv ("B", "2", 1), 0);
- TEST_VERIFY (environ != save_environ);
- TEST_COMPARE_STRING (environ[0], "A=1");
- TEST_COMPARE_STRING (environ[1], "B=2");
- TEST_COMPARE_STRING (environ[2], NULL);
- TEST_COMPARE_STRING (getenv ("PATH"), NULL);
- free (environ);
- environ = save_environ;
- TEST_COMPARE_STRING (getenv ("PATH"), original_path);
-}
-
-static int
-do_test (void)
-{
- original_path = getenv ("PATH");
- rewrite_environ ();
-
- /* Test again after reallocated the environment due to an initial
- setenv call. */
- TEST_COMPARE (setenv ("TST_SETENV_MALLOC", "1", 1), 0);
- TEST_VERIFY (environ != save_environ);
- rewrite_environ ();
-
- return 0;
-}
-
-#include <support/test-driver.c>

299
glibc-RHEL-75809-3.patch Normal file
View File

@ -0,0 +1,299 @@
commit 12b4a1fc6ecfc278a87159164bdf1d682deb18e2
Author: Florian Weimer <fweimer@redhat.com>
Date: Fri Jan 24 10:40:28 2025 +0100
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>
diff --git a/stdlib/Makefile b/stdlib/Makefile
index ff1418f5bb2ea5c9..f4dec9be46a573b9 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -275,6 +275,10 @@ tests := \
tst-canon-bz26341 \
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 d12401ca77cee5a7..20b0e1673c9557de 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 036f4274aa29b722..42b86fff1008bc81 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 0000000000000000..4241ad4c63ea2e33
--- /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 0000000000000000..b20be124902125e8
--- /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 0000000000000000..e77996a6cb0ac601
--- /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 0000000000000000..633ef7bda84eb2a8
--- /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 0000000000000000..c9b02844369207d9
--- /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
+ <https://www.gnu.org/licenses/>. */
+
+/* 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 <stdlib.h>
+#include <unistd.h>
+#include <support/check.h>
+#include <support/support.h>
+
+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 <support/test-driver.c>

7
glibc.spec
View File

@ -145,7 +145,7 @@ Version: %{glibcversion}
# - It allows using the Release number without the %%dist tag in the dependency
# generator to make the generated requires interchangeable between Rawhide
# and ELN (.elnYY < .fcXX).
%global baserelease 36
%global baserelease 37
Release: %{baserelease}%{?dist}
# Licenses:
@ -506,6 +506,8 @@ Patch188: glibc-upstream-2.39-145.patch
Patch189: glibc-upstream-2.39-146.patch
Patch190: glibc-RHEL-75809.patch
Patch191: glibc-RHEL-75555.patch
Patch192: glibc-RHEL-75809-2.patch
Patch193: glibc-RHEL-75809-3.patch
##############################################################################
# Continued list of core "glibc" package information:
@ -2501,6 +2503,9 @@ update_gconv_modules_cache ()
%endif
%changelog
* Fri Jan 24 2025 Florian Weimer <fweimer@redhat.com> - 2.39-37
- setenv: Rework free(environ) compatibility support (RHEL-75809)
* Thu Jan 23 2025 Florian Weimer <fweimer@redhat.com> - 2.39-36
- CVE-2025-0577: vDSO getrandom predictable randomness after fork (RHEL-75555)