* [review] Move _dl_open_check to its original place in dl_open_worker
@ 2019-10-31 19:20 Florian Weimer (Code Review)
2019-11-13 12:57 ` [review v2] " Florian Weimer (Code Review)
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Florian Weimer (Code Review) @ 2019-10-31 19:20 UTC (permalink / raw)
To: libc-alpha; +Cc: Florian Weimer
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(-)
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);
--
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I995a6cfb89f25d2b0cf5e606428c2a93eb48fc33
Gerrit-Change-Number: 473
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-MessageType: newchange
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [review v2] Move _dl_open_check to its original place in dl_open_worker
2019-10-31 19:20 [review] Move _dl_open_check to its original place in dl_open_worker Florian Weimer (Code Review)
@ 2019-11-13 12:57 ` Florian Weimer (Code Review)
2019-11-21 12:59 ` [review v3] " Carlos O'Donell (Code Review)
2019-11-27 20:30 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2 siblings, 0 replies; 4+ messages in thread
From: Florian Weimer (Code Review) @ 2019-11-13 12:57 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
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.
--
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I995a6cfb89f25d2b0cf5e606428c2a93eb48fc33
Gerrit-Change-Number: 473
Gerrit-PatchSet: 2
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Florian Weimer <fweimer@redhat.com>
Gerrit-Comment-Date: Wed, 13 Nov 2019 12:57:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 4+ messages in thread
* [review v3] Move _dl_open_check to its original place in dl_open_worker
2019-10-31 19:20 [review] Move _dl_open_check to its original place in dl_open_worker Florian Weimer (Code Review)
2019-11-13 12:57 ` [review v2] " Florian Weimer (Code Review)
@ 2019-11-21 12:59 ` Carlos O'Donell (Code Review)
2019-11-27 20:30 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2 siblings, 0 replies; 4+ messages in thread
From: Carlos O'Donell (Code Review) @ 2019-11-21 12:59 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
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
--
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I995a6cfb89f25d2b0cf5e606428c2a93eb48fc33
Gerrit-Change-Number: 473
Gerrit-PatchSet: 3
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Carlos O'Donell <carlos@redhat.com>
Gerrit-Reviewer: Florian Weimer <fweimer@redhat.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 12:59:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pushed] Move _dl_open_check to its original place in dl_open_worker
2019-10-31 19:20 [review] Move _dl_open_check to its original place in dl_open_worker Florian Weimer (Code Review)
2019-11-13 12:57 ` [review v2] " Florian Weimer (Code Review)
2019-11-21 12:59 ` [review v3] " Carlos O'Donell (Code Review)
@ 2019-11-27 20:30 ` Sourceware to Gerrit sync (Code Review)
2 siblings, 0 replies; 4+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-27 20:30 UTC (permalink / raw)
To: Florian Weimer, libc-alpha; +Cc: Carlos O'Donell
Sourceware to Gerrit sync has submitted this change.
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(-)
Approvals:
Carlos O'Donell: Looks good to me, approved
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 1051e22..df9f29a 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -622,6 +622,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);
@@ -702,12 +704,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);
--
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I995a6cfb89f25d2b0cf5e606428c2a93eb48fc33
Gerrit-Change-Number: 473
Gerrit-PatchSet: 6
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Carlos O'Donell <carlos@redhat.com>
Gerrit-Reviewer: Florian Weimer <fweimer@redhat.com>
Gerrit-MessageType: merged
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-27 20:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-31 19:20 [review] Move _dl_open_check to its original place in dl_open_worker Florian Weimer (Code Review)
2019-11-13 12:57 ` [review v2] " Florian Weimer (Code Review)
2019-11-21 12:59 ` [review v3] " Carlos O'Donell (Code Review)
2019-11-27 20:30 ` [pushed] " Sourceware to Gerrit sync (Code Review)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).