[COMMITTED] x86: Fix THREAD_SELF definition to avoid ld.so crash (bug 27004)
Commit Message
I reviewed and pushed this patch for Jakub. Tested with GCC 11
(x86-64), GCC 10 (x86-64), GCC 9 (i686.
Thanks,
Florian
8<------------------------------------------------------------------8<
From: Jakub Jelinek <jakub@redhat.com>
The previous definition of THREAD_SELF did not tell the compiler
that %fs (or %gs) usage is invalid for the !DL_LOOKUP_GSCOPE_LOCK
case in _dl_lookup_symbol_x. As a result, ld.so could try to use the
TCB before it was initialized.
As the comment in tls.h explains, asm volatile is undesirable here.
Using the __seg_fs (or __seg_gs) namespace does not interfere with
optimization, and expresses that THREAD_SELF is potentially trapping.
---
sysdeps/i386/nptl/tls.h | 7 ++++++-
sysdeps/x86_64/nptl/tls.h | 7 ++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
Comments
On 03/12/2020 09:51, Florian Weimer via Libc-alpha wrote:
> I reviewed and pushed this patch for Jakub. Tested with GCC 11
> (x86-64), GCC 10 (x86-64), GCC 9 (i686.
>
> Thanks,
> Florian
> 8<------------------------------------------------------------------8<
> From: Jakub Jelinek <jakub@redhat.com>
>
> The previous definition of THREAD_SELF did not tell the compiler
> that %fs (or %gs) usage is invalid for the !DL_LOOKUP_GSCOPE_LOCK
> case in _dl_lookup_symbol_x. As a result, ld.so could try to use the
> TCB before it was initialized.
>
> As the comment in tls.h explains, asm volatile is undesirable here.
> Using the __seg_fs (or __seg_gs) namespace does not interfere with
> optimization, and expresses that THREAD_SELF is potentially trapping.
>
> ---
> sysdeps/i386/nptl/tls.h | 7 ++++++-
> sysdeps/x86_64/nptl/tls.h | 7 ++++++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h
> index b74347bacd..ab98c24a02 100644
> --- a/sysdeps/i386/nptl/tls.h
> +++ b/sysdeps/i386/nptl/tls.h
> @@ -234,11 +234,16 @@ tls_fill_user_desc (union user_desc_init *desc,
> assignments like
> pthread_descr self = thread_self();
> do not get optimized away. */
> -# define THREAD_SELF \
> +# if __GNUC_PREREQ (6, 0)
> +# define THREAD_SELF \
> + (*(struct pthread *__seg_gs *) offsetof (struct pthread, header.self))
> +# else
> +# define THREAD_SELF \
> ({ struct pthread *__self; \
> asm ("movl %%gs:%c1,%0" : "=r" (__self) \
> : "i" (offsetof (struct pthread, header.self))); \
> __self;})
> +# endif
>
> /* Magic for libthread_db to know how to do THREAD_SELF. */
> # define DB_THREAD_SELF \
Do wee need to __GLIBC_PREREQ here? This is an internal header and the minimum
required gcc should be suffice here.
> diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
> index a08bf972de..ccb5f24d92 100644
> --- a/sysdeps/x86_64/nptl/tls.h
> +++ b/sysdeps/x86_64/nptl/tls.h
> @@ -180,11 +180,16 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
> assignments like
> pthread_descr self = thread_self();
> do not get optimized away. */
> -# define THREAD_SELF \
> +# if __GNUC_PREREQ (6, 0)
> +# define THREAD_SELF \
> + (*(struct pthread *__seg_fs *) offsetof (struct pthread, header.self))
> +# else
> +# define THREAD_SELF \
> ({ struct pthread *__self; \
> asm ("mov %%fs:%c1,%0" : "=r" (__self) \
> : "i" (offsetof (struct pthread, header.self))); \
> __self;})
> +# endif
>
> /* Magic for libthread_db to know how to do THREAD_SELF. */
> # define DB_THREAD_SELF_INCLUDE <sys/reg.h> /* For the FS constant. */
>
* Adhemerval Zanella via Libc-alpha:
>> diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h
>> index b74347bacd..ab98c24a02 100644
>> --- a/sysdeps/i386/nptl/tls.h
>> +++ b/sysdeps/i386/nptl/tls.h
>> @@ -234,11 +234,16 @@ tls_fill_user_desc (union user_desc_init *desc,
>> assignments like
>> pthread_descr self = thread_self();
>> do not get optimized away. */
>> -# define THREAD_SELF \
>> +# if __GNUC_PREREQ (6, 0)
>> +# define THREAD_SELF \
>> + (*(struct pthread *__seg_gs *) offsetof (struct pthread, header.self))
>> +# else
>> +# define THREAD_SELF \
>> ({ struct pthread *__self; \
>> asm ("movl %%gs:%c1,%0" : "=r" (__self) \
>> : "i" (offsetof (struct pthread, header.self))); \
>> __self;})
>> +# endif
>>
>> /* Magic for libthread_db to know how to do THREAD_SELF. */
>> # define DB_THREAD_SELF \
>
> Do wee need to __GLIBC_PREREQ here? This is an internal header and the minimum
> required gcc should be suffice here.
I think we can rework the header to use __seg_fs/__seg_gs throughout,
yes. I assumed we were still supporting GCC 5, which I see is not the
case.
Thanks,
Florian
@@ -234,11 +234,16 @@ tls_fill_user_desc (union user_desc_init *desc,
assignments like
pthread_descr self = thread_self();
do not get optimized away. */
-# define THREAD_SELF \
+# if __GNUC_PREREQ (6, 0)
+# define THREAD_SELF \
+ (*(struct pthread *__seg_gs *) offsetof (struct pthread, header.self))
+# else
+# define THREAD_SELF \
({ struct pthread *__self; \
asm ("movl %%gs:%c1,%0" : "=r" (__self) \
: "i" (offsetof (struct pthread, header.self))); \
__self;})
+# endif
/* Magic for libthread_db to know how to do THREAD_SELF. */
# define DB_THREAD_SELF \
@@ -180,11 +180,16 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
assignments like
pthread_descr self = thread_self();
do not get optimized away. */
-# define THREAD_SELF \
+# if __GNUC_PREREQ (6, 0)
+# define THREAD_SELF \
+ (*(struct pthread *__seg_fs *) offsetof (struct pthread, header.self))
+# else
+# define THREAD_SELF \
({ struct pthread *__self; \
asm ("mov %%fs:%c1,%0" : "=r" (__self) \
: "i" (offsetof (struct pthread, header.self))); \
__self;})
+# endif
/* Magic for libthread_db to know how to do THREAD_SELF. */
# define DB_THREAD_SELF_INCLUDE <sys/reg.h> /* For the FS constant. */