[Guile-commits] 02/06: Fix dangling pointers in `environ'.

[Ludovic Courtès]

civodul pushed a commit to branch main
in repository guile.

commit 84bf84032208e21d20ad13f3033d4fca3a512014
Author: Olivier Dion <olivier-dion@proton.me>
AuthorDate: Sat Apr 8 16:48:01 2023 -0400

    Fix dangling pointers in `environ'.
    
    When calling `environ', Guile set the global variable `environ' to a
    list allocated with the GC.  Strings in it are also allocated with the
    GC.
    
    However, if an user call the Scheme setenv() procedure, the resulting
    call to putenv() in libc might reallocate `environ' to a new pointer
    while copying sub-pointers owned by Guile in it.
    
    This results in the GC marking these strings for reclamation when they
    are actually still present in `environ'.  Thus, the values in the
    environment are now undefined.
    
    To fix this, Guile should only manipulate the `environ' using the
    standard libc functions.  This ensures that concurrent modification of
    it is safe in multi-threaded program.  Therefore, the procedure
    `environ' now call the libc clearenv() procedure to purge the
    environment.  Then, the desired values are put in `environ' using
    scm_putenv().  At the end, no GC allocated memory is put in `environ'.
    
    Also, since `environ' can be changed at anytime in a multi-thread
    program, emit a warning stipulating that the result is undefined
    behavior if multiple threads are created in the program.  Consider for
    example a thread iterating over `environ' while another one do a call to
    putenv().  The latter would do a realloc() on `environ' and thus the old
    array read by the former now contains garbage.
    
    On system where clearenv() is not present, an atomic store of NULL with
    sequential consistency to `environ' should be sufficient but see the
    NOTES of clearenv(3).
    
    * libguile/posix.c (scm_environ): Do not store GC allocated memory in
    environ.
    
    Signed-off-by: Ludovic Courtès <ludo@gnu.org>
---
 libguile/posix.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libguile/posix.c b/libguile/posix.c
index 4cf4ef383..4564cb33b 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -1755,11 +1755,30 @@ SCM_DEFINE (scm_environ, "environ", 0, 1, 0,
            "then the return value is unspecified.")
 #define FUNC_NAME s_scm_environ
 {
+  /* Accessing `environ' directly in a multi-threaded program is
+     undefined behavior since at anytime it could point to anything else
+     while reading it.  Not only that, but all accesses are protected by
+     an internal mutex of libc.  Thus, it is only truly safe to modify
+     the environment directly in a single-threaded program.  */
+  if (scm_ilength (scm_all_threads ()) != 1)
+    scm_display
+      (scm_from_latin1_string
+       ("warning: call to environ while multiple threads are running;\n"
+        "         further behavior unspecified.\n"),
+       scm_current_warning_port ());
+
   if (SCM_UNBNDP (env))
     return scm_makfromstrs (-1, environ);
   else
     {
-      environ = scm_i_allocate_string_pointers (env);
+      /* Arrange to not use GC-allocated storage for what goes into
+         'environ' as libc might reallocate it behind our back.  */
+      clearenv ();
+      while (!scm_is_null (env))
+        {
+          scm_putenv (scm_car (env));
+          env = scm_cdr (env);
+        }
       return SCM_UNSPECIFIED;
     }
 }