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 --- elf/dl-open.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) (limited to 'elf/dl-open.c') diff --git a/elf/dl-open.c b/elf/dl-open.c index 56f213323c..c23341be58 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); -- cgit 1.4.1