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