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