Re: [PATCH 4/8] nptl: Add rseq registration

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 12/7/21 18:31, Florian Weimer via Libc-alpha wrote:
> The rseq area is placed directly into struct pthread.  rseq
> registration failure is not treated as an error, so it is possible
> that threads run with inconsistent registration status.
> 
> <sys/rseq.h> is not yet installed as a public header.
> 
> Co-Authored-By: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
> v2: Use volatite access to cpu_id.  Drop csu/libc-tls.c spurious change.
> 
>   nptl/descr.h                                |   4 +
>   nptl/pthread_create.c                       |  13 +
>   sysdeps/nptl/dl-tls_init_tp.c               |   8 +-
>   sysdeps/unix/sysv/linux/Makefile            |   9 +-
>   sysdeps/unix/sysv/linux/aarch64/bits/rseq.h |  43 ++++
>   sysdeps/unix/sysv/linux/arm/bits/rseq.h     |  83 +++++++
>   sysdeps/unix/sysv/linux/bits/rseq.h         |  29 +++
>   sysdeps/unix/sysv/linux/mips/bits/rseq.h    |  62 +++++
>   sysdeps/unix/sysv/linux/powerpc/bits/rseq.h |  37 +++
>   sysdeps/unix/sysv/linux/rseq-internal.h     |  45 ++++
>   sysdeps/unix/sysv/linux/s390/bits/rseq.h    |  37 +++
>   sysdeps/unix/sysv/linux/sys/rseq.h          | 174 +++++++++++++
>   sysdeps/unix/sysv/linux/tst-rseq-nptl.c     | 260 ++++++++++++++++++++
>   sysdeps/unix/sysv/linux/tst-rseq.c          |  64 +++++
>   sysdeps/unix/sysv/linux/tst-rseq.h          |  57 +++++
>   sysdeps/unix/sysv/linux/x86/bits/rseq.h     |  30 +++
>   16 files changed, 952 insertions(+), 3 deletions(-)
>   create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
>   create mode 100644 sysdeps/unix/sysv/linux/arm/bits/rseq.h
>   create mode 100644 sysdeps/unix/sysv/linux/bits/rseq.h
>   create mode 100644 sysdeps/unix/sysv/linux/mips/bits/rseq.h
>   create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
>   create mode 100644 sysdeps/unix/sysv/linux/rseq-internal.h
>   create mode 100644 sysdeps/unix/sysv/linux/s390/bits/rseq.h
>   create mode 100644 sysdeps/unix/sysv/linux/sys/rseq.h
>   create mode 100644 sysdeps/unix/sysv/linux/tst-rseq-nptl.c
>   create mode 100644 sysdeps/unix/sysv/linux/tst-rseq.c
>   create mode 100644 sysdeps/unix/sysv/linux/tst-rseq.h
>   create mode 100644 sysdeps/unix/sysv/linux/x86/bits/rseq.h
> 
> diff --git a/nptl/descr.h b/nptl/descr.h
> index af2a6ab87a..92db305913 100644
> --- a/nptl/descr.h
> +++ b/nptl/descr.h
> @@ -34,6 +34,7 @@
>   #include <bits/types/res_state.h>
>   #include <kernel-features.h>
>   #include <tls-internal-struct.h>
> +#include <sys/rseq.h>
>   
>   #ifndef TCB_ALIGNMENT
>   # define TCB_ALIGNMENT 32
> @@ -406,6 +407,9 @@ struct pthread
>     /* Used on strsignal.  */
>     struct tls_internal_t tls_state;
>   
> +  /* rseq area registered with the kernel.  */
> +  struct rseq rseq_area;
> +
>     /* This member must be last.  */
>     char end_padding[];
>   
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index bad9eeb52f..ea0d79341e 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -32,6 +32,7 @@
>   #include <default-sched.h>
>   #include <futex-internal.h>
>   #include <tls-setup.h>
> +#include <rseq-internal.h>
>   #include "libioP.h"
>   #include <sys/single_threaded.h>
>   #include <version.h>
> @@ -366,6 +367,9 @@ start_thread (void *arg)
>     /* Initialize pointers to locale data.  */
>     __ctype_init ();
>   
> +  /* Register rseq TLS to the kernel.  */
> +  rseq_register_current_thread (pd);
> +
>   #ifndef __ASSUME_SET_ROBUST_LIST
>     if (__nptl_set_robust_list_avail)
>   #endif
> @@ -571,6 +575,15 @@ out:
>        process is really dead since 'clone' got passed the CLONE_CHILD_CLEARTID
>        flag.  The 'tid' field in the TCB will be set to zero.
>   
> +     rseq TLS is still registered at this point.  Rely on implicit
> +     unregistration performed by the kernel on thread teardown.  This is not a
> +     problem because the rseq TLS lives on the stack, and the stack outlives
> +     the thread.  If TCB allocation is ever changed, additional steps may be
> +     required, such as performing explicit rseq unregistration before
> +     reclaiming the rseq TLS area memory.  It is NOT sufficient to block
> +     signals because the kernel may write to the rseq area even without
> +     signals.
> +
>        The exit code is zero since in case all threads exit by calling
>        'pthread_exit' the exit status must be 0 (zero).  */
>     while (1)
> diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
> index ca494dd3a5..fedb876fdb 100644
> --- a/sysdeps/nptl/dl-tls_init_tp.c
> +++ b/sysdeps/nptl/dl-tls_init_tp.c
> @@ -21,6 +21,7 @@
>   #include <list.h>
>   #include <pthreadP.h>
>   #include <tls.h>
> +#include <rseq-internal.h>
>   
>   #ifndef __ASSUME_SET_ROBUST_LIST
>   bool __nptl_set_robust_list_avail;
> @@ -57,11 +58,12 @@ __tls_pre_init_tp (void)
>   void
>   __tls_init_tp (void)
>   {
> +  struct pthread *pd = THREAD_SELF;
> +
>     /* Set up thread stack list management.  */
> -  list_add (&THREAD_SELF->list, &GL (dl_stack_user));
> +  list_add (&pd->list, &GL (dl_stack_user));
>   
>      /* Early initialization of the TCB.   */
> -   struct pthread *pd = THREAD_SELF;
>      pd->tid = INTERNAL_SYSCALL_CALL (set_tid_address, &pd->tid);
>      THREAD_SETMEM (pd, specific[0], &pd->specific_1stblock[0]);
>      THREAD_SETMEM (pd, user_stack, true);
> @@ -90,6 +92,8 @@ __tls_init_tp (void)
>         }
>     }
>   
> +  rseq_register_current_thread (pd);
> +
>     /* Set initial thread's stack block from 0 up to __libc_stack_end.
>        It will be bigger than it actually is, but for unwind.c/pt-longjmp.c
>        purposes this is good enough.  */
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 29c6c78f98..eb0f5fc021 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -131,7 +131,10 @@ ifeq ($(have-GLIBC_2.27)$(build-shared),yesyes)
>   tests += tst-ofdlocks-compat
>   endif
>   
> -tests-internal += tst-sigcontext-get_pc
> +tests-internal += \
> +  tst-rseq \
> +  tst-sigcontext-get_pc \
> +  # tests-internal
>   
>   tests-time64 += \
>     tst-adjtimex-time64 \
> @@ -357,4 +360,8 @@ endif
>   
>   ifeq ($(subdir),nptl)
>   tests += tst-align-clone tst-getpid1
> +
> +# tst-rseq-nptl is an internal test because it requires a definition of
> +# __NR_rseq from the internal system call list.
> +tests-internal += tst-rseq-nptl
>   endif
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> new file mode 100644
> index 0000000000..9ba92725c7
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> @@ -0,0 +1,43 @@
> +/* Restartable Sequences Linux aarch64 architecture header.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* RSEQ_SIG is a signature required before each abort handler code.
> +
> +   It is a 32-bit value that maps to actual architecture code compiled
> +   into applications and libraries.  It needs to be defined for each
> +   architecture.  When choosing this value, it needs to be taken into
> +   account that generating invalid instructions may have ill effects on
> +   tools like objdump, and may also have impact on the CPU speculative
> +   execution efficiency in some cases.
> +
> +   aarch64 -mbig-endian generates mixed endianness code vs data:
> +   little-endian code and big-endian data.  Ensure the RSEQ_SIG signature
> +   matches code endianness.  */
> +
> +#define RSEQ_SIG_CODE  0xd428bc00  /* BRK #0x45E0.  */
> +
> +#ifdef __AARCH64EB__
> +# define RSEQ_SIG_DATA 0x00bc28d4  /* BRK #0x45E0.  */
> +#else
> +# define RSEQ_SIG_DATA RSEQ_SIG_CODE
> +#endif
> +
> +#define RSEQ_SIG       RSEQ_SIG_DATA
> diff --git a/sysdeps/unix/sysv/linux/arm/bits/rseq.h b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> new file mode 100644
> index 0000000000..0542b26f6a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> @@ -0,0 +1,83 @@
> +/* Restartable Sequences Linux arm architecture header.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/*
> +   RSEQ_SIG is a signature required before each abort handler code.
> +
> +   It is a 32-bit value that maps to actual architecture code compiled
> +   into applications and libraries.  It needs to be defined for each
> +   architecture.  When choosing this value, it needs to be taken into
> +   account that generating invalid instructions may have ill effects on
> +   tools like objdump, and may also have impact on the CPU speculative
> +   execution efficiency in some cases.
> +
> +   - ARM little endian
> +
> +   RSEQ_SIG uses the udf A32 instruction with an uncommon immediate operand
> +   value 0x5de3.  This traps if user-space reaches this instruction by mistake,
> +   and the uncommon operand ensures the kernel does not move the instruction
> +   pointer to attacker-controlled code on rseq abort.
> +
> +   The instruction pattern in the A32 instruction set is:
> +
> +   e7f5def3    udf    #24035    ; 0x5de3
> +
> +   This translates to the following instruction pattern in the T16 instruction
> +   set:
> +
> +   little endian:
> +   def3        udf    #243      ; 0xf3
> +   e7f5        b.n    <7f5>
> +
> +   - ARMv6+ big endian (BE8):
> +
> +   ARMv6+ -mbig-endian generates mixed endianness code vs data: little-endian
> +   code and big-endian data.  The data value of the signature needs to have its
> +   byte order reversed to generate the trap instruction:
> +
> +   Data: 0xf3def5e7
> +
> +   Translates to this A32 instruction pattern:
> +
> +   e7f5def3    udf    #24035    ; 0x5de3
> +
> +   Translates to this T16 instruction pattern:
> +
> +   def3        udf    #243      ; 0xf3
> +   e7f5        b.n    <7f5>
> +
> +   - Prior to ARMv6 big endian (BE32):
> +
> +   Prior to ARMv6, -mbig-endian generates big-endian code and data
> +   (which match), so the endianness of the data representation of the
> +   signature should not be reversed.  However, the choice between BE32
> +   and BE8 is done by the linker, so we cannot know whether code and
> +   data endianness will be mixed before the linker is invoked.  So rather
> +   than try to play tricks with the linker, the rseq signature is simply
> +   data (not a trap instruction) prior to ARMv6 on big endian.  This is
> +   why the signature is expressed as data (.word) rather than as
> +   instruction (.inst) in assembler.  */
> +
> +#ifdef __ARMEB__
> +# define RSEQ_SIG    0xf3def5e7      /* udf    #24035    ; 0x5de3 (ARMv6+) */
> +#else
> +# define RSEQ_SIG    0xe7f5def3      /* udf    #24035    ; 0x5de3 */
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/bits/rseq.h b/sysdeps/unix/sysv/linux/bits/rseq.h
> new file mode 100644
> index 0000000000..46cf5d1c74
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/rseq.h
> @@ -0,0 +1,29 @@
> +/* Restartable Sequences architecture header.  Stub version.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* RSEQ_SIG is a signature required before each abort handler code.
> +
> +   It is a 32-bit value that maps to actual architecture code compiled
> +   into applications and libraries.  It needs to be defined for each
> +   architecture.  When choosing this value, it needs to be taken into
> +   account that generating invalid instructions may have ill effects on
> +   tools like objdump, and may also have impact on the CPU speculative
> +   execution efficiency in some cases.  */
> diff --git a/sysdeps/unix/sysv/linux/mips/bits/rseq.h b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
> new file mode 100644
> index 0000000000..a9defee568
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
> @@ -0,0 +1,62 @@
> +/* Restartable Sequences Linux mips architecture header.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* RSEQ_SIG is a signature required before each abort handler code.
> +
> +   It is a 32-bit value that maps to actual architecture code compiled
> +   into applications and libraries.  It needs to be defined for each
> +   architecture.  When choosing this value, it needs to be taken into
> +   account that generating invalid instructions may have ill effects on
> +   tools like objdump, and may also have impact on the CPU speculative
> +   execution efficiency in some cases.
> +
> +   RSEQ_SIG uses the break instruction.  The instruction pattern is:
> +
> +   On MIPS:
> +        0350000d        break     0x350
> +
> +   On nanoMIPS:
> +        00100350        break     0x350
> +
> +   On microMIPS:
> +        0000d407        break     0x350
> +
> +   For nanoMIPS32 and microMIPS, the instruction stream is encoded as
> +   16-bit halfwords, so the signature halfwords need to be swapped
> +   accordingly for little-endian.  */
> +
> +#if defined (__nanomips__)
> +# ifdef __MIPSEL__
> +#  define RSEQ_SIG      0x03500010
> +# else
> +#  define RSEQ_SIG      0x00100350
> +# endif
> +#elif defined (__mips_micromips)
> +# ifdef __MIPSEL__
> +#  define RSEQ_SIG      0xd4070000
> +# else
> +#  define RSEQ_SIG      0x0000d407
> +# endif
> +#elif defined (__mips__)
> +# define RSEQ_SIG       0x0350000d
> +#else
> +/* Unknown MIPS architecture.  */
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h b/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
> new file mode 100644
> index 0000000000..05b3cf7b8f
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
> @@ -0,0 +1,37 @@
> +/* Restartable Sequences Linux powerpc architecture header.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_RSEQ_H
> +# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
> +#endif
> +
> +/* RSEQ_SIG is a signature required before each abort handler code.
> +
> +   It is a 32-bit value that maps to actual architecture code compiled
> +   into applications and libraries.  It needs to be defined for each
> +   architecture.  When choosing this value, it needs to be taken into
> +   account that generating invalid instructions may have ill effects on
> +   tools like objdump, and may also have impact on the CPU speculative
> +   execution efficiency in some cases.
> +
> +   RSEQ_SIG uses the following trap instruction:
> +
> +   powerpc-be:    0f e5 00 0b           twui   r5,11
> +   powerpc64-le:  0b 00 e5 0f           twui   r5,11
> +   powerpc64-be:  0f e5 00 0b           twui   r5,11  */
> +
> +#define RSEQ_SIG        0x0fe5000b
> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
> new file mode 100644
> index 0000000000..909f547825
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h
> @@ -0,0 +1,45 @@
> +/* Restartable Sequences internal API.  Linux implementation.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef RSEQ_INTERNAL_H
> +#define RSEQ_INTERNAL_H
> +
> +#include <sysdep.h>
> +#include <errno.h>
> +#include <kernel-features.h>
> +#include <stdio.h>
> +#include <sys/rseq.h>
> +
> +#ifdef RSEQ_SIG
> +static inline void
> +rseq_register_current_thread (struct pthread *self)
> +{
> +  int ret = INTERNAL_SYSCALL_CALL (rseq,
> +                                   &self->rseq_area, sizeof (self->rseq_area),
> +                                   0, RSEQ_SIG);
> +  if (INTERNAL_SYSCALL_ERROR_P (ret))
> +    THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);

Why can't we just leave it as the kernel did when it failed the syscall? 
  It looks like we'll only end up shadowing UNINITIALIZED all the time 
and it may cause issues if linux decides to use -2 for some other 
purpose in future.

Siddhesh