[review] Move _dl_open_check to its original place in dl_open_worker

Message ID gerrit.1572549639000.I995a6cfb89f25d2b0cf5e606428c2a93eb48fc33@gnutoolchain-gerrit.osci.io
State Superseded
Headers

Commit Message

Simon Marchi (Code Review) Oct. 31, 2019, 7:20 p.m. UTC
  Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/473
......................................................................

Move _dl_open_check to its original place in dl_open_worker

This reverts the non-test change from commit d0093c5cefb7f7a4143f
("Call _dl_open_check after relocation [BZ #24259]"), given that
the underlying bug has been fixed properly in commit 61b74477fa7f63
("Remove all loaded objects if dlopen fails, ignoring NODELETE
[BZ #20839]").

Tested on x86-64-linux-gnu, with and without --enable-cet.

Change-Id: I995a6cfb89f25d2b0cf5e606428c2a93eb48fc33
---
M elf/dl-open.c
1 file changed, 2 insertions(+), 6 deletions(-)
  

Comments

Simon Marchi (Code Review) Nov. 13, 2019, 12:57 p.m. UTC | #1
Florian Weimer has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/473
......................................................................


Patch Set 2:

This change is ready for review.
  
Simon Marchi (Code Review) Nov. 21, 2019, 12:59 p.m. UTC | #2
Carlos O'Donell has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/473
......................................................................


Patch Set 3: Code-Review+2

(2 comments)

When we correctly remove NODELETE objects we can call _dl_open_check early.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

| --- elf/dl-open.c
| +++ elf/dl-open.c
  

Patch

diff --git a/elf/dl-open.c b/elf/dl-open.c
index 7322c5d..126cf9c 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -605,6 +605,8 @@ 
   _dl_debug_state ();
   LIBC_PROBE (map_complete, 3, args->nsid, r, new);
 
+  _dl_open_check (new);
+
   /* Print scope information.  */
   if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_SCOPES))
     _dl_show_scope (new, 0);
@@ -685,12 +687,6 @@ 
 	_dl_relocate_object (l, l->l_scope, reloc_mode, 0);
     }
 
-  /* NB: Workaround for [BZ #20839] which doesn't remove the NODELETE
-     object when _dl_open_check throws an exception.  Move it after
-     relocation to avoid leaving the NODELETE object mapped without
-     relocation.  */
-  _dl_open_check (new);
-
   /* This only performs the memory allocations.  The actual update of
      the scopes happens below, after failure is impossible.  */
   resize_scopes (new);