5f7b84
commit f7649d5780aa4682393b9daedd653e4d9c12784c
5f7b84
Author: Florian Weimer <fweimer@redhat.com>
5f7b84
Date:   Fri Dec 13 10:23:10 2019 +0100
5f7b84
5f7b84
    dlopen: Do not block signals
5f7b84
    
5f7b84
    Blocking signals causes issues with certain anti-malware solutions
5f7b84
    which rely on an unblocked SIGSYS signal for system calls they
5f7b84
    intercept.
5f7b84
    
5f7b84
    This reverts commit a2e8aa0d9ea648068d8be52dd7b15f1b6a008e23
5f7b84
    ("Block signals during the initial part of dlopen") and adds
5f7b84
    comments related to async signal safety to active_nodelete and
5f7b84
    its caller.
5f7b84
    
5f7b84
    Note that this does not make lazy binding async-signal-safe with regards
5f7b84
    to dlopen.  It merely avoids introducing new async-signal-safety hazards
5f7b84
    as part of the NODELETE changes.
5f7b84
    
5f7b84
    Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
5f7b84
    Reviewed-by: Carlos O'Donell <carlos@redhat.com>
5f7b84
5f7b84
diff --git a/elf/dl-open.c b/elf/dl-open.c
5f7b84
index a382bfae8aa3a2f8..d834b89754d2b073 100644
5f7b84
--- a/elf/dl-open.c
5f7b84
+++ b/elf/dl-open.c
5f7b84
@@ -34,7 +34,6 @@
5f7b84
 #include <atomic.h>
5f7b84
 #include <libc-internal.h>
5f7b84
 #include <array_length.h>
5f7b84
-#include <internal-signals.h>
5f7b84
 
5f7b84
 #include <dl-dst.h>
5f7b84
 #include <dl-prop.h>
5f7b84
@@ -53,10 +52,6 @@ struct dl_open_args
5f7b84
   /* Namespace ID.  */
5f7b84
   Lmid_t nsid;
5f7b84
 
5f7b84
-  /* Original signal mask.  Used for unblocking signal handlers before
5f7b84
-     running ELF constructors.  */
5f7b84
-  sigset_t original_signal_mask;
5f7b84
-
5f7b84
   /* Original value of _ns_global_scope_pending_adds.  Set by
5f7b84
      dl_open_worker.  Only valid if nsid is a real namespace
5f7b84
      (non-negative).  */
5f7b84
@@ -446,6 +441,9 @@ activate_nodelete (struct link_map *new)
5f7b84
 	  _dl_debug_printf ("activating NODELETE for %s [%lu]\n",
5f7b84
 			    l->l_name, l->l_ns);
5f7b84
 
5f7b84
+	/* The flag can already be true at this point, e.g. a signal
5f7b84
+	   handler may have triggered lazy binding and set NODELETE
5f7b84
+	   status immediately.  */
5f7b84
 	l->l_nodelete_active = true;
5f7b84
 
5f7b84
 	/* This is just a debugging aid, to indicate that
5f7b84
@@ -520,16 +518,12 @@ dl_open_worker (void *a)
5f7b84
   if (new == NULL)
5f7b84
     {
5f7b84
       assert (mode & RTLD_NOLOAD);
5f7b84
-      __libc_signal_restore_set (&args->original_signal_mask);
5f7b84
       return;
5f7b84
     }
5f7b84
 
5f7b84
   if (__glibc_unlikely (mode & __RTLD_SPROF))
5f7b84
-    {
5f7b84
-      /* This happens only if we load a DSO for 'sprof'.  */
5f7b84
-      __libc_signal_restore_set (&args->original_signal_mask);
5f7b84
-      return;
5f7b84
-    }
5f7b84
+    /* This happens only if we load a DSO for 'sprof'.  */
5f7b84
+    return;
5f7b84
 
5f7b84
   /* This object is directly loaded.  */
5f7b84
   ++new->l_direct_opencount;
5f7b84
@@ -565,7 +559,6 @@ dl_open_worker (void *a)
5f7b84
 
5f7b84
       assert (_dl_debug_initialize (0, args->nsid)->r_state == RT_CONSISTENT);
5f7b84
 
5f7b84
-      __libc_signal_restore_set (&args->original_signal_mask);
5f7b84
       return;
5f7b84
     }
5f7b84
 
5f7b84
@@ -709,6 +702,12 @@ dl_open_worker (void *a)
5f7b84
      All memory allocations for new objects must have happened
5f7b84
      before.  */
5f7b84
 
5f7b84
+  /* Finalize the NODELETE status first.  This comes before
5f7b84
+     update_scopes, so that lazy binding will not see pending NODELETE
5f7b84
+     state for newly loaded objects.  There is a compiler barrier in
5f7b84
+     update_scopes which ensures that the changes from
5f7b84
+     activate_nodelete are visible before new objects show up in the
5f7b84
+     local scope.  */
5f7b84
   activate_nodelete (new);
5f7b84
 
5f7b84
   /* Second stage after resize_scopes: Actually perform the scope
5f7b84
@@ -742,10 +741,6 @@ dl_open_worker (void *a)
5f7b84
   if (mode & RTLD_GLOBAL)
5f7b84
     add_to_global_resize (new);
5f7b84
 
5f7b84
-  /* Unblock signals.  Data structures are now consistent, and
5f7b84
-     application code may run.  */
5f7b84
-  __libc_signal_restore_set (&args->original_signal_mask);
5f7b84
-
5f7b84
   /* Run the initializer functions of new objects.  Temporarily
5f7b84
      disable the exception handler, so that lazy binding failures are
5f7b84
      fatal.  */
5f7b84
@@ -835,10 +830,6 @@ no more namespaces available for dlmopen()"));
5f7b84
   args.argv = argv;
5f7b84
   args.env = env;
5f7b84
 
5f7b84
-  /* Recursive lazy binding during manipulation of the dynamic loader
5f7b84
-     structures may result in incorrect behavior.  */
5f7b84
-  __libc_signal_block_all (&args.original_signal_mask);
5f7b84
-
5f7b84
   struct dl_exception exception;
5f7b84
   int errcode = _dl_catch_exception (&exception, dl_open_worker, &args);
5f7b84
 
5f7b84
@@ -879,16 +870,10 @@ no more namespaces available for dlmopen()"));
5f7b84
 
5f7b84
 	  _dl_close_worker (args.map, true);
5f7b84
 
5f7b84
-	  /* Restore the signal mask.  In the success case, this
5f7b84
-	     happens inside dl_open_worker.  */
5f7b84
-	  __libc_signal_restore_set (&args.original_signal_mask);
5f7b84
-
5f7b84
 	  /* All l_nodelete_pending objects should have been deleted
5f7b84
 	     at this point, which is why it is not necessary to reset
5f7b84
 	     the flag here.  */
5f7b84
 	}
5f7b84
-      else
5f7b84
-	__libc_signal_restore_set (&args.original_signal_mask);
5f7b84
 
5f7b84
       assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);
5f7b84