public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Florian Weimer <fw@deneb.enyo.de>
To: Joseph Myers <joseph@codesourcery.com>
Cc: <libc-alpha@sourceware.org>,
	 Christian Brauner <christian.brauner@ubuntu.com>,
	Samuel Thibault <samuel.thibault@ens-lyon.org>
Subject: Re: [pushed] Block signals during the initial part of dlopen
Date: Thu, 28 Nov 2019 19:08:00 -0000	[thread overview]
Message-ID: <87o8wv7rag.fsf@mid.deneb.enyo.de> (raw)
In-Reply-To: <87y2vz7s7j.fsf@mid.deneb.enyo.de> (Florian Weimer's message of "Thu, 28 Nov 2019 19:48:16 +0100")

* Florian Weimer:

> * Joseph Myers:
>
>> On Wed, 27 Nov 2019, Sourceware to Gerrit sync (Code Review) wrote:
>>
>>> Block signals during the initial part of dlopen
>>> 
>>> Lazy binding in a signal handler that interrupts a dlopen 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.
>>
>> This change (commit a2e8aa0d9ea648068d8be52dd7b15f1b6a008e23) breaks the 
>> build for i686-gnu (unfortunately this was after a syntax error was 
>> introduced in build-many-glibcs.py so my bots quietly died on re-execing 
>> with the syntax error, and so didn't get to find this build failure).
>>
>> There are a series of errors linking ld.so starting with:
>>
>> /scratch/jmyers/glibc/many9/install/compilers/i686-gnu/lib/gcc/i686-glibc-gnu/9.2.1/../../../../i686-glibc-gnu/bin/ld:
>> /scratch/jmyers/glibc/many9/build/glibcs/i686-gnu/glibc/libc_pic.a(libc_fatal.os):
>> in function `__GI___libc_fatal':
>> /scratch/jmyers/glibc/many9/src/glibc/libio/../sysdeps/posix/libc_fatal.c:161:
>> multiple definition of `__libc_fatal';
>> /scratch/jmyers/glibc/many9/build/glibcs/i686-gnu/glibc/elf/dl-allobjs.os:/scratch/jmyers/glibc/many9/src/glibc/elf/dl-minimal.c:188:
>> first defined here
>>
>> and leading up to:
>>
>> make[3]: ***
>> [/scratch/jmyers/glibc/many9/build/glibcs/i686-gnu/glibc/elf/librtld.map]
>> Error 1
>
> I failed to test the later commits using build-many-glibcs.py because
> I deemed them more straightforward.
>
> The patch below should fix it.

Posted too soon.  The localplt update was missing.

8<------------------------------------------------------------------8<
hurd: Add stub implementation of __sigprocmask to the dynamic loader

The actual __sigprocmask cannot be built for the loader since it
depends on per-thread state.

Fixes commit a2e8aa0d9ea648068d8be52dd7b15f1b6a008e23 ("Block signals
during the initial part of dlopen").

diff --git a/sysdeps/mach/hurd/Versions b/sysdeps/mach/hurd/Versions
index f69d5fef67..57d45e6d0d 100644
--- a/sysdeps/mach/hurd/Versions
+++ b/sysdeps/mach/hurd/Versions
@@ -41,6 +41,6 @@ ld {
 
     # functions that must be shared with libc
     __access_noerrno; __libc_read; __libc_write; __libc_lseek64;
-    __libc_lock_self0;
+    __libc_lock_self0; __sigprocmask;
   }
 }
diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c
index 719d603f44..e35ce69c4a 100644
--- a/sysdeps/mach/hurd/dl-sysdep.c
+++ b/sysdeps/mach/hurd/dl-sysdep.c
@@ -693,6 +693,15 @@ _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
 		     timeout, notify);
 }
 
+/* This function does nothing and will be interposed with the actual
+   implementation once is libc is loaded.  Before that, manipulating
+   the signal mask does not make sense because no signal handlers have
+   been installed.  */
+int weak_function
+__sigprocmask (int how, const sigset_t *set, sigset_t *oset)
+{
+  return 0;
+}
 
 void
 _dl_show_auxv (void)
diff --git a/sysdeps/mach/hurd/i386/localplt.data b/sysdeps/mach/hurd/i386/localplt.data
index a5b5241b84..b3f5cefeea 100644
--- a/sysdeps/mach/hurd/i386/localplt.data
+++ b/sysdeps/mach/hurd/i386/localplt.data
@@ -41,6 +41,7 @@ ld.so: __strtoul_internal
 #ld.so: _exit
 ld.so: abort
 ld.so: _hurd_intr_rpc_mach_msg
+ld.so: __sigprocmask
 ld.so: __errno_location
 # rtld_hidden is currently disabled to avoid having to special-case the
 # functions above which do need a PLT.  These are thus currently expected.

      reply	other threads:[~2019-11-28 19:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-31 19:21 [review] " Florian Weimer (Code Review)
2019-11-06 13:45 ` Christian Brauner (Code Review)
2019-11-06 15:17 ` 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:31 ` [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 [this message]

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o8wv7rag.fsf@mid.deneb.enyo.de \
    --to=fw@deneb.enyo.de \
    --cc=christian.brauner@ubuntu.com \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    --cc=samuel.thibault@ens-lyon.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).