From f8ed116aa574435c6e28260f21963233682d3b57 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Fri, 13 Dec 2019 10:18:46 +0100 Subject: 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 Reviewed-by: Carlos O'Donell --- include/link.h | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) (limited to 'include') 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 -- cgit v1.1