From: "Florian Weimer (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: libc-alpha@sourceware.org
Cc: Florian Weimer <fweimer@redhat.com>
Subject: [review] Block signals during the initial part of dlopen
Date: Thu, 31 Oct 2019 15:20:41 -0400 [thread overview]
Message-ID: <gerrit.1572549639000.Iad079080ebe7442c13313ba11dc2797953faef35@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: gerrit.1572549639000.Iad079080ebe7442c13313ba11dc2797953faef35@gnutoolchain-gerrit.osci.io
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/472
......................................................................
Block signals during the initial part of dlopen
Lazy binding during dlopen from a signal handler sees intermediate
dynamic linker state. This has likely been always unsafe, but with
the new pending NODELETE state, this is clearly incorrect. Other
threads are excluded via the loader lock, but the current thread is
not. Blocking signals until right before ELF constructors run is
the safe thing to do.
Change-Id: Iad079080ebe7442c13313ba11dc2797953faef35
---
M elf/dl-open.c
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 9b42f2b..7322c5d 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -34,6 +34,7 @@
#include <atomic.h>
#include <libc-internal.h>
#include <array_length.h>
+#include <internal-signals.h>
#include <dl-dst.h>
#include <dl-prop.h>
@@ -52,6 +53,10 @@
/* Namespace ID. */
Lmid_t nsid;
+ /* Original signal mask. Used for unblocking signal handlers before
+ running ELF constructors. */
+ sigset_t original_signal_mask;
+
/* Original value of _ns_global_scope_pending_adds. Set by
dl_open_worker. Only valid if nsid is a real namespace
(non-negative). */
@@ -510,12 +515,16 @@
if (new == NULL)
{
assert (mode & RTLD_NOLOAD);
+ __libc_signal_restore_set (&args->original_signal_mask);
return;
}
if (__glibc_unlikely (mode & __RTLD_SPROF))
- /* This happens only if we load a DSO for 'sprof'. */
- return;
+ {
+ /* This happens only if we load a DSO for 'sprof'. */
+ __libc_signal_restore_set (&args->original_signal_mask);
+ return;
+ }
/* This object is directly loaded. */
++new->l_direct_opencount;
@@ -551,6 +560,7 @@
assert (_dl_debug_initialize (0, args->nsid)->r_state == RT_CONSISTENT);
+ __libc_signal_restore_set (&args->original_signal_mask);
return;
}
@@ -730,6 +740,10 @@
if (mode & RTLD_GLOBAL)
add_to_global_resize (new);
+ /* Unblock signals. Data structures are now consistent, and
+ application code may run. */
+ __libc_signal_restore_set (&args->original_signal_mask);
+
/* Run the initializer functions of new objects. Temporarily
disable the exception handler, so that lazy binding failures are
fatal. */
@@ -819,6 +833,10 @@
args.argv = argv;
args.env = env;
+ /* Recursive lazy binding during manipulation of the dynamic loader
+ structures may result in incorrect behavior. */
+ __libc_signal_block_all (&args.original_signal_mask);
+
struct dl_exception exception;
int errcode = _dl_catch_exception (&exception, dl_open_worker, &args);
@@ -859,10 +877,16 @@
_dl_close_worker (args.map, true);
+ /* Restore the signal mask. In the success case, this
+ 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. */
}
+ else
+ __libc_signal_restore_set (&args.original_signal_mask);
assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);
--
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: Iad079080ebe7442c13313ba11dc2797953faef35
Gerrit-Change-Number: 472
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-MessageType: newchange
next parent reply other threads:[~2019-10-31 19:21 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-31 19:20 Florian Weimer (Code Review) [this message]
2019-11-06 13:45 ` [review] Block signals during the initial part of dlopen Christian Brauner (Code Review)
2019-11-06 15:16 ` Christian Brauner (Code Review)
2019-11-07 11:18 ` Christian Brauner (Code Review)
2019-11-07 11:43 ` Florian Weimer (Code Review)
2019-11-13 13:09 ` [review v2] " Florian Weimer (Code Review)
2019-11-15 16:02 ` [review v3] " Florian Weimer (Code Review)
2019-11-15 16:08 ` Christian Brauner (Code Review)
2019-11-27 20:30 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-28 18:04 ` Joseph Myers
2019-11-28 18:48 ` Florian Weimer
2019-11-28 19:08 ` Florian Weimer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=gerrit.1572549639000.Iad079080ebe7442c13313ba11dc2797953faef35@gnutoolchain-gerrit.osci.io \
--to=gerrit@gnutoolchain-gerrit.osci.io \
--cc=fweimer@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).