Upstream patches for fallout from dlopen NODELETE changes (#1778344, #1778366)

glibc-dlopen-nodelete-fixes-2.patch

@@ -0,0 +1,296 @@
+Author: Florian Weimer <fweimer@redhat.com>
+Date:   Thu Dec 5 13:32:42 2019 +0100
+
+    dlopen: Rework handling of pending NODELETE status
+    
+    To avoid a read-modify-write cycle on the l_nodelete field, this
+    commit introduces two flags for active NODELETE status (irrevocable)
+    and pending NODELETE status (revocable until activate_nodelete) is
+    invoked.  As a result, NODELETE processing in dlopen does not
+    introduce further reasons why lazy binding from signal handlers
+    is unsafe during dlopen, and a subsequent commit can remove signal
+    blocking from dlopen.
+
+diff --git a/elf/dl-close.c b/elf/dl-close.c
+index e35a62daf6fa98a1..df1df6fb298b2319 100644
+--- a/elf/dl-close.c
++++ b/elf/dl-close.c
+@@ -197,7 +197,7 @@ _dl_close_worker (struct link_map *map, bool force)
+       /* Check whether this object is still used.  */
+       if (l->l_type == lt_loaded
+ 	  && l->l_direct_opencount == 0
+-	  && l->l_nodelete != link_map_nodelete_active
++	  && !l->l_nodelete_active
+ 	  /* See CONCURRENCY NOTES in cxa_thread_atexit_impl.c to know why
+ 	     acquire is sufficient and correct.  */
+ 	  && atomic_load_acquire (&l->l_tls_dtor_count) == 0
+@@ -279,8 +279,7 @@ _dl_close_worker (struct link_map *map, bool force)
+ 
+       if (!used[i])
+ 	{
+-	  assert (imap->l_type == lt_loaded
+-		  && imap->l_nodelete != link_map_nodelete_active);
++	  assert (imap->l_type == lt_loaded && !imap->l_nodelete_active);
+ 
+ 	  /* Call its termination function.  Do not do it for
+ 	     half-cooked objects.  Temporarily disable exception
+@@ -830,7 +829,7 @@ _dl_close (void *_map)
+      before we took the lock. There is no way to detect this (see below)
+      so we proceed assuming this isn't the case.  First see whether we
+      can remove the object at all.  */
+-  if (__glibc_unlikely (map->l_nodelete == link_map_nodelete_active))
++  if (__glibc_unlikely (map->l_nodelete_active))
+     {
+       /* Nope.  Do nothing.  */
+       __rtld_lock_unlock_recursive (GL(dl_load_lock));
+diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
+index 99846918c3453694..759b45a2c977175a 100644
+--- a/elf/dl-lookup.c
++++ b/elf/dl-lookup.c
+@@ -187,6 +187,28 @@ enter_unique_sym (struct unique_sym *table, size_t size,
+   table[idx].map = map;
+ }
+ 
++/* Mark MAP as NODELETE according to the lookup mode in FLAGS.  During
++   initial relocation, NODELETE state is pending only.  */
++static void
++mark_nodelete (struct link_map *map, int flags)
++{
++  if (flags & DL_LOOKUP_FOR_RELOCATE)
++    map->l_nodelete_pending = true;
++  else
++    map->l_nodelete_active = true;
++}
++
++/* Return true if MAP is marked as NODELETE according to the lookup
++   mode in FLAGS> */
++static bool
++is_nodelete (struct link_map *map, int flags)
++{
++  /* Non-pending NODELETE always counts.  Pending NODELETE only counts
++     during initial relocation processing.  */
++  return map->l_nodelete_active
++    || ((flags & DL_LOOKUP_FOR_RELOCATE) && map->l_nodelete_pending);
++}
++
+ /* Utility function for do_lookup_x. Lookup an STB_GNU_UNIQUE symbol
+    in the unique symbol table, creating a new entry if necessary.
+    Return the matching symbol in RESULT.  */
+@@ -311,8 +333,7 @@ do_lookup_unique (const char *undef_name, uint_fast32_t new_hash,
+       enter_unique_sym (entries, size,
+                         new_hash, strtab + sym->st_name, sym, map);
+ 
+-      if (map->l_type == lt_loaded
+-	  && map->l_nodelete == link_map_nodelete_inactive)
++      if (map->l_type == lt_loaded && !is_nodelete (map, flags))
+ 	{
+ 	  /* Make sure we don't unload this object by
+ 	     setting the appropriate flag.  */
+@@ -320,10 +341,7 @@ do_lookup_unique (const char *undef_name, uint_fast32_t new_hash,
+ 	    _dl_debug_printf ("\
+ marking %s [%lu] as NODELETE due to unique symbol\n",
+ 			      map->l_name, map->l_ns);
+-	  if (flags & DL_LOOKUP_FOR_RELOCATE)
+-	    map->l_nodelete = link_map_nodelete_pending;
+-	  else
+-	    map->l_nodelete = link_map_nodelete_active;
++	  mark_nodelete (map, flags);
+ 	}
+     }
+   ++tab->n_elements;
+@@ -586,7 +604,7 @@ add_dependency (struct link_map *undef_map, struct link_map *map, int flags)
+      dependencies may pick an dependency which can be dlclose'd, but
+      such IFUNC resolvers are undefined anyway.  */
+   assert (map->l_type == lt_loaded);
+-  if (map->l_nodelete != link_map_nodelete_inactive)
++  if (is_nodelete (map, flags))
+     return 0;
+ 
+   struct link_map_reldeps *l_reldeps
+@@ -694,17 +712,16 @@ add_dependency (struct link_map *undef_map, struct link_map *map, int flags)
+ 
+       /* Redo the NODELETE check, as when dl_load_lock wasn't held
+ 	 yet this could have changed.  */
+-      if (map->l_nodelete != link_map_nodelete_inactive)
++      if (is_nodelete (map, flags))
+ 	goto out;
+ 
+       /* If the object with the undefined reference cannot be removed ever
+ 	 just make sure the same is true for the object which contains the
+ 	 definition.  */
+-      if (undef_map->l_type != lt_loaded
+-	  || (undef_map->l_nodelete != link_map_nodelete_inactive))
++      if (undef_map->l_type != lt_loaded || is_nodelete (map, flags))
+ 	{
+ 	  if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_BINDINGS)
+-	      && map->l_nodelete == link_map_nodelete_inactive)
++	      && !is_nodelete (map, flags))
+ 	    {
+ 	      if (undef_map->l_name[0] == '\0')
+ 		_dl_debug_printf ("\
+@@ -716,11 +733,7 @@ marking %s [%lu] as NODELETE due to reference to %s [%lu]\n",
+ 				  map->l_name, map->l_ns,
+ 				  undef_map->l_name, undef_map->l_ns);
+ 	    }
+-
+-	  if (flags & DL_LOOKUP_FOR_RELOCATE)
+-	    map->l_nodelete = link_map_nodelete_pending;
+-	  else
+-	    map->l_nodelete = link_map_nodelete_active;
++	  mark_nodelete (map, flags);
+ 	  goto out;
+ 	}
+ 
+@@ -746,17 +759,14 @@ marking %s [%lu] as NODELETE due to reference to %s [%lu]\n",
+ 		 cannot be unloaded.  This is semantically the correct
+ 		 behavior.  */
+ 	      if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_BINDINGS)
+-		  && map->l_nodelete == link_map_nodelete_inactive)
++		  && !is_nodelete (map, flags))
+ 		_dl_debug_printf ("\
+ marking %s [%lu] as NODELETE due to memory allocation failure\n",
+ 				  map->l_name, map->l_ns);
+-	      if (flags & DL_LOOKUP_FOR_RELOCATE)
+-		/* In case of non-lazy binding, we could actually
+-		   report the memory allocation error, but for now, we
+-		   use the conservative approximation as well. */
+-		map->l_nodelete = link_map_nodelete_pending;
+-	      else
+-		map->l_nodelete = link_map_nodelete_active;
++	      /* In case of non-lazy binding, we could actually report
++		 the memory allocation error, but for now, we use the
++		 conservative approximation as well.  */
++	      mark_nodelete (map, flags);
+ 	      goto out;
+ 	    }
+ 	  else
+diff --git a/elf/dl-open.c b/elf/dl-open.c
+index 56f213323ce5cf65..c23341be5892fb12 100644
+--- a/elf/dl-open.c
++++ b/elf/dl-open.c
+@@ -440,13 +440,17 @@ activate_nodelete (struct link_map *new)
+      NODELETE status for objects outside the local scope.  */
+   for (struct link_map *l = GL (dl_ns)[new->l_ns]._ns_loaded; l != NULL;
+        l = l->l_next)
+-    if (l->l_nodelete == link_map_nodelete_pending)
++    if (l->l_nodelete_pending)
+       {
+ 	if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_FILES))
+ 	  _dl_debug_printf ("activating NODELETE for %s [%lu]\n",
+ 			    l->l_name, l->l_ns);
+ 
+-	l->l_nodelete = link_map_nodelete_active;
++	l->l_nodelete_active = true;
++
++	/* This is just a debugging aid, to indicate that
++	   activate_nodelete has run for this map.  */
++	l->l_nodelete_pending = false;
+       }
+ }
+ 
+@@ -549,10 +553,10 @@ dl_open_worker (void *a)
+       if (__glibc_unlikely (mode & RTLD_NODELETE))
+ 	{
+ 	  if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_FILES)
+-	      && new->l_nodelete == link_map_nodelete_inactive)
++	      && !new->l_nodelete_active)
+ 	    _dl_debug_printf ("marking %s [%lu] as NODELETE\n",
+ 			      new->l_name, new->l_ns);
+-	  new->l_nodelete = link_map_nodelete_active;
++	  new->l_nodelete_active = true;
+ 	}
+ 
+       /* Finalize the addition to the global scope.  */
+@@ -568,7 +572,7 @@ dl_open_worker (void *a)
+   /* Schedule NODELETE marking for the directly loaded object if
+      requested.  */
+   if (__glibc_unlikely (mode & RTLD_NODELETE))
+-    new->l_nodelete = link_map_nodelete_pending;
++    new->l_nodelete_pending = true;
+ 
+   /* Load that object's dependencies.  */
+   _dl_map_object_deps (new, NULL, 0, 0,
+@@ -683,7 +687,7 @@ dl_open_worker (void *a)
+ 	      _dl_start_profile ();
+ 
+ 	      /* Prevent unloading the object.  */
+-	      GL(dl_profile_map)->l_nodelete = link_map_nodelete_active;
++	      GL(dl_profile_map)->l_nodelete_active = true;
+ 	    }
+ 	}
+       else
+@@ -882,9 +886,9 @@ no more namespaces available for dlmopen()"));
+ 	     happens inside dl_open_worker.  */
+ 	  __libc_signal_restore_set (&args.original_signal_mask);
+ 
+-	  /* All link_map_nodelete_pending objects should have been
+-	     deleted at this point, which is why it is not necessary
+-	     to reset the flag here.  */
++	  /* All l_nodelete_pending objects should have been deleted
++	     at this point, which is why it is not necessary to reset
++	     the flag here.  */
+ 	}
+       else
+ 	__libc_signal_restore_set (&args.original_signal_mask);
+diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h
+index 075683d729447a40..ea4e304bef282a6f 100644
+--- a/elf/get-dynamic-info.h
++++ b/elf/get-dynamic-info.h
+@@ -164,7 +164,7 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
+     {
+       l->l_flags_1 = info[VERSYMIDX (DT_FLAGS_1)]->d_un.d_val;
+       if (l->l_flags_1 & DF_1_NODELETE)
+-	l->l_nodelete = link_map_nodelete_pending;
++	l->l_nodelete_pending = true;
+ 
+       /* Only DT_1_SUPPORTED_MASK bits are supported, and we would like
+ 	 to assert this, but we can't. Users have been setting
+diff --git a/include/link.h b/include/link.h
+index 2e771e433a36e6cd..2acc8363802d6d64 100644
+--- a/include/link.h
++++ b/include/link.h
+@@ -79,22 +79,6 @@ struct r_search_path_struct
+     int malloced;
+   };
+ 
+-/* Type used by the l_nodelete member.  */
+-enum link_map_nodelete
+-{
+- /* This link map can be deallocated.  */
+- link_map_nodelete_inactive = 0, /* Zero-initialized in _dl_new_object.  */
+-
+- /* This link map cannot be deallocated.  */
+- link_map_nodelete_active,
+-
+- /* This link map cannot be deallocated after dlopen has succeded.
+-    dlopen turns this into link_map_nodelete_active.  dlclose treats
+-    this intermediate state as link_map_nodelete_active.  */
+- link_map_nodelete_pending,
+-};
+-
+-
+ /* Structure describing a loaded shared object.  The `l_next' and `l_prev'
+    members form a chain of all the shared objects loaded at startup.
+ 
+@@ -218,10 +202,17 @@ struct link_map
+ 				       freed, ie. not allocated with
+ 				       the dummy malloc in ld.so.  */
+ 
+-    /* Actually of type enum link_map_nodelete.  Separate byte due to
+-       a read in add_dependency in elf/dl-lookup.c outside the loader
+-       lock.  Only valid for l_type == lt_loaded.  */
+-    unsigned char l_nodelete;
++    /* NODELETE status of the map.  Only valid for maps of type
++       lt_loaded.  Lazy binding sets l_nodelete_active directly,
++       potentially from signal handlers.  Initial loading of an
++       DF_1_NODELETE object set l_nodelete_pending.  Relocation may
++       set l_nodelete_pending as well.  l_nodelete_pending maps are
++       promoted to l_nodelete_active status in the final stages of
++       dlopen, prior to calling ELF constructors.  dlclose only
++       refuses to unload l_nodelete_active maps, the pending status is
++       ignored.  */
++    bool l_nodelete_active;
++    bool l_nodelete_pending;
+ 
+ #include <link_map.h>
+ 

glibc-dlopen-nodelete-fixes-3.patch

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

glibc.spec

@@ -87,7 +87,7 @@
 Summary: The GNU libc libraries
 Name: glibc
 Version: %{glibcversion}
-Release: 23%{?dist}
+Release: 24%{?dist}
 
 # In general, GPLv2+ is used by programs, LGPLv2+ is used for
 # libraries.
@@ -158,7 +158,9 @@ Patch17: glibc-cs-path.patch
 Patch18: glibc-c-utf8-locale.patch
 Patch23: glibc-python3.patch
 Patch29: glibc-fedora-nsswitch.patch
-Patch30: glibc-rh1778344.patch
+Patch30: glibc-dlopen-nodelete-fixes-1.patch
+Patch31: glibc-dlopen-nodelete-fixes-2.patch
+Patch32: glibc-dlopen-nodelete-fixes-3.patch
 
 ##############################################################################
 # Continued list of core "glibc" package information:
@@ -2042,6 +2044,9 @@ fi
 %files -f compat-libpthread-nonshared.filelist -n compat-libpthread-nonshared
 
 %changelog
+* Thu Dec  5 2019 Florian Weimer <fweimer@redhat.com> - 2.30.9000-24
+- Upstream patches for fallout from dlopen NODELETE changes (#1778344, #1778366)
+
 * Wed Dec 04 2019 Patsy Franklin <patsy@redhat.com> - 2.30.9000-23
 - Auto-sync with upstream branch master,
   commit ec138c67cbda8b5826a0a2a7ba456408117996dc.