Fix BZ 14333

1 files changed, 48 insertions, 20 deletions

diff --git a/stdlib/cxa_finalize.c b/stdlib/cxa_finalize.c
index aa0a70cb58..23fc6dcafb 100644
--- a/stdlib/cxa_finalize.c
+++ b/stdlib/cxa_finalize.c
@@ -17,7 +17,6 @@
 
 #include <assert.h>
 #include <stdlib.h>
-#include <atomic.h>
 #include "exit.h"
 #include <fork.h>
 #include <sysdep.h>
@@ -31,36 +30,64 @@ __cxa_finalize (void *d)
 {
   struct exit_function_list *funcs;
 
+  __libc_lock_lock (__exit_funcs_lock);
+
  restart:
   for (funcs = __exit_funcs; funcs; funcs = funcs->next)
     {
       struct exit_function *f;
 
       for (f = &funcs->fns[funcs->idx - 1]; f >= &funcs->fns[0]; --f)
-	{
-	  void (*cxafn) (void *arg, int status);
-	  void *cxaarg;
+	if ((d == NULL || d == f->func.cxa.dso_handle) && f->flavor == ef_cxa)
+	  {
+	    const uint64_t check = __new_exitfn_called;
+	    void (*cxafn) (void *arg, int status) = f->func.cxa.fn;
+	    void *cxaarg = f->func.cxa.arg;
+
+	    /* We don't want to run this cleanup more than once.  The Itanium
+	       C++ ABI requires that multiple calls to __cxa_finalize not
+	       result in calling termination functions more than once.  One
+	       potential scenario where that could happen is with a concurrent
+	       dlclose and exit, where the running dlclose must at some point
+	       release the list lock, an exiting thread may acquire it, and
+	       without setting flavor to ef_free, might re-run this destructor
+	       which could result in undefined behaviour.  Therefore we must
+	       set flavor to ef_free to avoid calling this destructor again.
+	       Note that the concurrent exit must also take the dynamic loader
+	       lock (for library finalizer processing) and therefore will
+	       block while dlclose completes the processing of any in-progress
+	       exit functions. Lastly, once we release the list lock for the
+	       entry marked ef_free, we must not read from that entry again
+	       since it may have been reused by the time we take the list lock
+	       again.  Lastly the detection of new registered exit functions is
+	       based on a monotonically incrementing counter, and there is an
+	       ABA if between the unlock to run the exit function and the
+	       re-lock after completion the user registers 2^64 exit functions,
+	       the implementation will not detect this and continue without
+	       executing any more functions.
 
-	  if ((d == NULL || d == f->func.cxa.dso_handle)
-	      /* We don't want to run this cleanup more than once.  */
-	      && (cxafn = f->func.cxa.fn,
-		  cxaarg = f->func.cxa.arg,
-		  ! catomic_compare_and_exchange_bool_acq (&f->flavor, ef_free,
-							   ef_cxa)))
-	    {
-	      uint64_t check = __new_exitfn_called;
+	       One minor issue remains: A registered exit function that is in
+	       progress by a call to dlclose() may not completely finish before
+	       the next registered exit function is run. This may, according to
+	       some readings of POSIX violate the requirement that functions
+	       run in effective LIFO order.  This should probably be fixed in a
+	       future implementation to ensure the functions do not run in
+	       parallel.  */
+	    f->flavor = ef_free;
 
 #ifdef PTR_DEMANGLE
-	      PTR_DEMANGLE (cxafn);
+	    PTR_DEMANGLE (cxafn);
 #endif
-	      cxafn (cxaarg, 0);
+	    /* Unlock the list while we call a foreign function.  */
+	    __libc_lock_unlock (__exit_funcs_lock);
+	    cxafn (cxaarg, 0);
+	    __libc_lock_lock (__exit_funcs_lock);
 
-	      /* It is possible that that last exit function registered
-		 more exit functions.  Start the loop over.  */
-	      if (__glibc_unlikely (check != __new_exitfn_called))
-		goto restart;
-	    }
-	}
+	    /* It is possible that that last exit function registered
+	       more exit functions.  Start the loop over.  */
+	    if (__glibc_unlikely (check != __new_exitfn_called))
+	      goto restart;
+	  }
     }
 
   /* Also remove the quick_exit handlers, but do not call them.  */
@@ -79,4 +106,5 @@ __cxa_finalize (void *d)
   if (d != NULL)
     UNREGISTER_ATFORK (d);
 #endif
+  __libc_lock_unlock (__exit_funcs_lock);
 }