dlopen: Rework handling of pending NODELETE status

Commit a2e8aa0d9ea648068d8be52dd7b15f1b6a008e23 ("Block signals during
the initial part of dlopen") was deemed necessary because of
read-modify-write operations like the one in  add_dependency in
elf/dl-lookup.c.  In the old code, we check for any kind of NODELETE
status and bail out:

      /* 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)
	goto out;

And then set pending status (during relocation):

	  if (flags & DL_LOOKUP_FOR_RELOCATE)
	    map->l_nodelete = link_map_nodelete_pending;
	  else
	    map->l_nodelete = link_map_nodelete_active;

If a signal arrives during relocation and the signal handler, through
lazy binding, adds a global scope dependency on the same map, it will
set map->l_nodelete to link_map_nodelete_active.  This will be
overwritten with link_map_nodelete_pending by the dlopen relocation
code.

To avoid such problems in relation to the l_nodelete member, 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.

This does not address pre-existing issues (unrelated to the NODELETE
changes) which make lazy binding in a signal handler during dlopen
unsafe, such as the use of malloc in both cases.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>

1 files changed, 11 insertions, 20 deletions

diff --git a/include/link.h b/include/link.h
index 2e771e4..2acc836 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>