[1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v10)

Message ID 2022553041.20966.1559249801435.JavaMail.zimbra@efficios.com
State Superseded
Headers

Commit Message

Mathieu Desnoyers May 30, 2019, 8:56 p.m. UTC
  ----- On May 29, 2019, at 11:45 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> ----- On May 27, 2019, at 3:27 PM, Mathieu Desnoyers
> mathieu.desnoyers@efficios.com wrote:
> 
>> ----- On May 27, 2019, at 7:19 AM, Florian Weimer fweimer@redhat.com wrote:
>> 
> 
> [...]
> 
>>> 
>>> Furthermore, the reference to ELF constructors is misleading.  I believe
>>> the code you added to __libc_start_main to initialize __rseq_handled and
>>> register __seq_abi with the kernel runs *after* ELF constructors have
>>> executed (and not at all if the main program is written in Go, alas).
>>> All initialization activity for the shared case needs to happen in
>>> elf/rtld.c or called from there, probably as part of the security
>>> initialization code or thereabouts.
>> 
>> in elf/rtld.c:dl_main() we have the following code:
>> 
>>  /* We do not initialize any of the TLS functionality unless any of the
>>     initial modules uses TLS.  This makes dynamic loading of modules with
>>     TLS impossible, but to support it requires either eagerly doing setup
>>     now or lazily doing it later.  Doing it now makes us incompatible with
>>     an old kernel that can't perform TLS_INIT_TP, even if no TLS is ever
>>     used.  Trying to do it lazily is too hairy to try when there could be
>>     multiple threads (from a non-TLS-using libpthread).  */
>>  bool was_tls_init_tp_called = tls_init_tp_called;
>>  if (tcbp == NULL)
>>    tcbp = init_tls ();
>> 
>> If I understand your point correctly, I should move the rseq_init() and
>> rseq_register_current_thread() for the SHARED case just after this
>> initialization, otherwise calling those from LIBC_START_MAIN() is too
>> late and it runs after initial modules constructors (or not at all for
>> Go). However, this means glibc will start using TLS internally. I'm
>> concerned that this is not quite in line with the above comment which
>> states that TLS is not initialized if no initial modules use TLS.
>> 
>> For the !SHARED use-case, if my understanding is correct, I should keep
>> rseq_init() and rseq_register_current_thread() calls within LIBC_START_MAIN().
> 
> I've moved the rseq initialization for SHARED case to the very end of
> elf/rtld.c:init_tls(), and get the following error on make check:
> 
> Generating locale am_ET.UTF-8: this might take a while...
> Inconsistency detected by ld.so: get-dynamic-info.h: 143: elf_get_dynamic_info:
> Assertion `info[DT_FLAGS] == NULL || (info[DT_FLAGS]->d_un.d_val &
> ~DF_BIND_NOW) == 0' failed!
> Charmap: "UTF-8" Inputfile: "am_ET" Outputdir: "am_ET.UTF-8" failed
> /bin/sh: 4: cannot create
> /home/efficios/git/glibc-build/localedata/am_ET.UTF-8/LC_CTYPE.test-result:
> Directory nonexistent
> 
> This error goes away if I comment out the call to rseq_register_current_thread
> (),
> which touches the __rseq_abi __thread variable and issues a system call.
> 
> Currently, the __rseq_abi __thread variable is within
> sysdeps/unix/sysv/linux/rseq-sym.c, which is added to the
> sysdep_routines within sysdeps/unix/sysv/linux/Makefile. I
> suspect it may need to be moved elsewhere.
> 
> Any thoughts on how to solve this ?

I found that it's because touching a __thread variable from
ld-linux-x86-64.so.2 ends up setting the DF_STATIC_TLS flag
for that .so, which is really not expected.

Even if I tweak the assert to make it more lenient there,
touching the __thread variable ends up triggering a SIGFPE.

So rather than touching the TLS from ld-linux-x86-64.so.2,
I've rather experimented with moving the rseq initialization
for both SHARED and !SHARED cases to a library constructor
within libc.so.

Are you aware of any downside to this approach ?



Thanks,

Mathieu
  

Comments

Florian Weimer May 31, 2019, 8:06 a.m. UTC | #1
* Mathieu Desnoyers:

> I found that it's because touching a __thread variable from
> ld-linux-x86-64.so.2 ends up setting the DF_STATIC_TLS flag
> for that .so, which is really not expected.
>
> Even if I tweak the assert to make it more lenient there,
> touching the __thread variable ends up triggering a SIGFPE.

Sorry, I got distracted at this critical juncture.  Yes, I forgot that
there isn't TLS support in the dynamic loader today.

> So rather than touching the TLS from ld-linux-x86-64.so.2,
> I've rather experimented with moving the rseq initialization
> for both SHARED and !SHARED cases to a library constructor
> within libc.so.
>
> Are you aware of any downside to this approach ?

The information whether the kernel supports rseq would not be available
to IFUNC resolvers.  And in some cases, ELF constructors for application
libraries could run before the libc.so.6 constructor, so applications
would see a transition from lack of kernel support to kernel support.

> +static
> +__attribute__ ((constructor))
> +void __rseq_libc_init (void)
> +{
> +  rseq_init ();
> +  /* Register rseq ABI to the kernel.   */
> +  (void) rseq_register_current_thread ();
> +}

I think the call to rseq_init (and the __rseq_handled variable) should
still be part of the dynamic loader.  Otherwise there could be confusion
about whether glibc handles the registration (due the constructor
ordering issue).

Thanks,
Florian
  
Mathieu Desnoyers May 31, 2019, 2:48 p.m. UTC | #2
----- On May 31, 2019, at 4:06 AM, Florian Weimer fweimer@redhat.com wrote:

> * Mathieu Desnoyers:
> 
>> I found that it's because touching a __thread variable from
>> ld-linux-x86-64.so.2 ends up setting the DF_STATIC_TLS flag
>> for that .so, which is really not expected.
>>
>> Even if I tweak the assert to make it more lenient there,
>> touching the __thread variable ends up triggering a SIGFPE.
> 
> Sorry, I got distracted at this critical juncture.  Yes, I forgot that
> there isn't TLS support in the dynamic loader today.
> 
>> So rather than touching the TLS from ld-linux-x86-64.so.2,
>> I've rather experimented with moving the rseq initialization
>> for both SHARED and !SHARED cases to a library constructor
>> within libc.so.
>>
>> Are you aware of any downside to this approach ?
> 
> The information whether the kernel supports rseq would not be available
> to IFUNC resolvers.  And in some cases, ELF constructors for application
> libraries could run before the libc.so.6 constructor, so applications
> would see a transition from lack of kernel support to kernel support.
> 
>> +static
>> +__attribute__ ((constructor))
>> +void __rseq_libc_init (void)
>> +{
>> +  rseq_init ();
>> +  /* Register rseq ABI to the kernel.   */
>> +  (void) rseq_register_current_thread ();
>> +}
> 
> I think the call to rseq_init (and the __rseq_handled variable) should
> still be part of the dynamic loader.  Otherwise there could be confusion
> about whether glibc handles the registration (due the constructor
> ordering issue).

Let's break this down into the various sub-issues involved:

1) How early do we need to setup rseq ? Should it be setup before:
   - LD_PRELOAD .so constructors ?
     - Without circular dependency,
     - With circular dependency,
   - audit libraries initialization ?
   - IFUNC resolvers ?
   - other callbacks ?
   - memory allocator calls ?

We may end up in a situation where we need memory allocation to be setup
in order to initialize TLS before rseq can be registered for the main
thread. I suspect we will end up needing a fallbacks which always work
for the few cases that would try to use rseq too early in dl/libc startup.

2) Do we need to setup __rseq_handled and __rseq_abi at the same stage of
   startup, or is it OK to setup __rseq_handled before __rseq_abi ?

3) Which shared object owns __rseq_handled and __rseq_abi ?
   - libc.so ?
   - ld-linux-*.so.2 ?
   - Should both symbols be owned by the same .so ?
   - What about the !SHARED case ? I think this would end up in libc.a in all cases.

4) Inability to touch a TLS variable (__rseq_abi) from ld-linux-*.so.2
   - Should we extend the dynamic linker to allow such TLS variable to be
     accessed ? If so, how much effort is required ?
   - Can we find an alternative way to initialize rseq early during
     dl init stages while still performing the TLS access from a function
     implemented within libc.so ?

So far, I got rseq to be initialized before LD_PRELOADed library constructors
by doing the initialization in a constructor within libc.so. I don't particularly
like this approach, because the constructor order is not guaranteed.

One possible solution would be to somehow expose a rseq initialization function
symbol from libc.so, look it up from ld-linux-*.so.2, and invoke it after libc.so
has been loaded. It would end up being similar to a constructor, but with a
fixed invocation order.

I'm just not sure we have everything we need to do this in ld-linux-*.so.2
init stages.

Thoughts ?

Thanks,

Mathieu
  
Florian Weimer May 31, 2019, 3:46 p.m. UTC | #3
* Mathieu Desnoyers:

> Let's break this down into the various sub-issues involved:
>
> 1) How early do we need to setup rseq ? Should it be setup before:
>    - LD_PRELOAD .so constructors ?
>      - Without circular dependency,
>      - With circular dependency,
>    - audit libraries initialization ?
>    - IFUNC resolvers ?
>    - other callbacks ?
>    - memory allocator calls ?
>
> We may end up in a situation where we need memory allocation to be setup
> in order to initialize TLS before rseq can be registered for the main
> thread. I suspect we will end up needing a fallbacks which always work
> for the few cases that would try to use rseq too early in dl/libc startup.

I think the answer to that depends on whether it's okay to have an
observable transition from “no rseq kernel support” to “kernel supports
rseq”.

> 2) Do we need to setup __rseq_handled and __rseq_abi at the same stage of
>    startup, or is it OK to setup __rseq_handled before __rseq_abi ?

I think we should be able to set __rseq_handle early, even if we can
perform the rseq area registration later.  (The distinction does not
matter if the registration needs to be performed early as well.)

Setting __rseq_handle in ld.so is easy if the variable is defined in
ld.so, which is not a problem at all.

> 3) Which shared object owns __rseq_handled and __rseq_abi ?
>    - libc.so ?
>    - ld-linux-*.so.2 ?
>    - Should both symbols be owned by the same .so ?

I think we can pick whatever works, based on the requirements from (1).
It's an implementation detail (altough it currently becomes part of the
ABI for weird reasons, but the choice itself is arbitrary).

>    - What about the !SHARED case ? I think this would end up in libc.a
>    in all cases.

Correct.

> 4) Inability to touch a TLS variable (__rseq_abi) from ld-linux-*.so.2
>    - Should we extend the dynamic linker to allow such TLS variable to be
>      accessed ? If so, how much effort is required ?
>    - Can we find an alternative way to initialize rseq early during
>      dl init stages while still performing the TLS access from a function
>      implemented within libc.so ?

This is again related to the answer for (1).  There are various hacks we
could implement to make the initialization invisible (e.g., computing
the address of the variable using the equivalent of dlsym, after loading
all the initial objects and before starting relocation).  If it's not
too hard to add TLS support to ld.so, we can consider that as well.
(The allocation side should be pretty easy, relocation support it could
be more tricky.)

> So far, I got rseq to be initialized before LD_PRELOADed library
> constructors by doing the initialization in a constructor within
> libc.so. I don't particularly like this approach, because the
> constructor order is not guaranteed.

Right.

> One possible solution would be to somehow expose a rseq initialization
> function symbol from libc.so, look it up from ld-linux-*.so.2, and
> invoke it after libc.so has been loaded. It would end up being similar
> to a constructor, but with a fixed invocation order.

This would still expose lack of rseq support to IFUNC resolvers
initially.  I don't know if this is a problem (again, it comes down to
(1) above).  There is a school of thought that you can't reference
__rseq_abi from an IFUNC resolver because it needs a relocation.

Thanks,
Florian
  

Patch

diff --git a/csu/libc-start.c b/csu/libc-start.c
index 5d9c3675fa..9755ed5467 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -22,6 +22,7 @@ 
 #include <ldsodefs.h>
 #include <exit-thread.h>
 #include <libc-internal.h>
+#include <rseq-internal.h>
 
 #include <elf/dl-tunables.h>
 
@@ -81,6 +82,14 @@  apply_irel (void)
 }
 #endif
 
+static
+__attribute__ ((constructor))
+void __rseq_libc_init (void)
+{
+  rseq_init ();
+  /* Register rseq ABI to the kernel.   */
+  (void) rseq_register_current_thread ();
+}
 
 #ifdef LIBC_START_MAIN
 # ifdef LIBC_START_DISABLE_INLINE