[RFC,1/4] glibc: Perform rseq(2) registration at C startup and thread creation (v6)

  Register rseq(2) TLS for each thread (including main), and unregister
for each thread (excluding main). "rseq" stands for Restartable
Sequences.

See the rseq(2) man page proposed here:
  https://lkml.org/lkml/2018/9/19/647

This patch is based on glibc commit a502c5294. The rseq(2) system call
was merged into Linux 4.18.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Dave Watson <davejwatson@fb.com>
CC: Paul Turner <pjt@google.com>
CC: Rich Felker <dalias@libc.org>
CC: libc-alpha@sourceware.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
---
Changes since v1:
- Move __rseq_refcount to an extra field at the end of __rseq_abi to
  eliminate one symbol.

  All libraries/programs which try to register rseq (glibc,
  early-adopter applications, early-adopter libraries) should use the
  rseq refcount. It becomes part of the ABI within a user-space
  process, but it's not part of the ABI shared with the kernel per se.

- Restructure how this code is organized so glibc keeps building on
  non-Linux targets.

- Use non-weak symbol for __rseq_abi.

- Move rseq registration/unregistration implementation into its own
  nptl/rseq.c compile unit.

- Move __rseq_abi symbol under GLIBC_2.29.

Changes since v2:
- Move __rseq_refcount to its own symbol, which is less ugly than
  trying to play tricks with the rseq uapi.
- Move __rseq_abi from nptl to csu (C start up), so it can be used
  across glibc, including memory allocator and sched_getcpu(). The
  __rseq_refcount symbol is kept in nptl, because there is no reason
  to use it elsewhere in glibc.

Changes since v3:
- Set __rseq_refcount TLS to 1 on register/set to 0 on unregister
  because glibc is the first/last user.
- Unconditionally register/unregister rseq at thread start/exit, because
  glibc is the first/last user.
- Add missing abilist items.
- Rebase on glibc master commit a502c5294.
- Add NEWS entry.

Changes since v4:
- Do not use "weak" symbols for __rseq_abi and __rseq_refcount. Based on
  "System V Application Binary Interface", weak only affects the link
  editor, not the dynamic linker.
- Install a new sys/rseq.h system header on Linux, which contains the
  RSEQ_SIG definition, __rseq_abi declaration and __rseq_refcount
  declaration. Move those definition/declarations from rseq-internal.h
  to the installed sys/rseq.h header.
- Considering that rseq is only available on Linux, move csu/rseq.c to
  sysdeps/unix/sysv/linux/rseq-sym.c.
- Move __rseq_refcount from nptl/rseq.c to
  sysdeps/unix/sysv/linux/rseq-sym.c, so it is only defined on Linux.
- Move both ABI definitions for __rseq_abi and __rseq_refcount to
  sysdeps/unix/sysv/linux/Versions, so they only appear on Linux.
- Document __rseq_abi and __rseq_refcount volatile.
- Document the RSEQ_SIG signature define.
- Move registration functions from rseq.c to rseq-internal.h static
  inline functions. Introduce empty stubs in misc/rseq-internal.h,
  which can be overridden by architecture code in
  sysdeps/unix/sysv/linux/rseq-internal.h.
- Rename __rseq_register_current_thread and __rseq_unregister_current_thread
  to rseq_register_current_thread and rseq_unregister_current_thread,
  now that those are only visible as internal static inline functions.
- Invoke rseq_register_current_thread() from libc-start.c LIBC_START_MAIN
  rather than nptl init, so applications not linked against
  libpthread.so have rseq registered for their main() thread. Note that
  it is invoked separately for SHARED and !SHARED builds.

Changes since v5:
- Replace __rseq_refcount by __rseq_lib_abi, which contains two
  uint32_t: register_state and refcount. The "register_state" field
  allows inhibiting rseq registration from signal handlers nested on top
  of glibc registration and occuring after rseq unregistration by glibc.
- Introduce enum rseq_register_state, which contains the states allowed
  for the struct rseq_lib_abi register_state field.
---
 NEWS                                          |  6 ++
 csu/libc-start.c                              | 12 ++-
 misc/Makefile                                 |  3 +-
 misc/rseq-internal.h                          | 34 +++++++
 nptl/pthread_create.c                         |  9 ++
 sysdeps/unix/sysv/linux/Makefile              |  5 +-
 sysdeps/unix/sysv/linux/Versions              |  4 +
 sysdeps/unix/sysv/linux/aarch64/libc.abilist  |  2 +
 sysdeps/unix/sysv/linux/alpha/libc.abilist    |  2 +
 sysdeps/unix/sysv/linux/arm/libc.abilist      |  2 +
 sysdeps/unix/sysv/linux/hppa/libc.abilist     |  2 +
 sysdeps/unix/sysv/linux/i386/libc.abilist     |  2 +
 sysdeps/unix/sysv/linux/ia64/libc.abilist     |  2 +
 .../sysv/linux/m68k/coldfire/libc.abilist     |  2 +
 .../unix/sysv/linux/m68k/m680x0/libc.abilist  |  2 +
 .../unix/sysv/linux/microblaze/libc.abilist   |  2 +
 .../sysv/linux/mips/mips32/fpu/libc.abilist   |  2 +
 .../sysv/linux/mips/mips32/nofpu/libc.abilist |  2 +
 .../sysv/linux/mips/mips64/n32/libc.abilist   |  2 +
 .../sysv/linux/mips/mips64/n64/libc.abilist   |  2 +
 sysdeps/unix/sysv/linux/nios2/libc.abilist    |  2 +
 .../linux/powerpc/powerpc32/fpu/libc.abilist  |  2 +
 .../powerpc/powerpc32/nofpu/libc.abilist      |  2 +
 .../linux/powerpc/powerpc64/libc-le.abilist   |  2 +
 .../sysv/linux/powerpc/powerpc64/libc.abilist |  2 +
 .../unix/sysv/linux/riscv/rv64/libc.abilist   |  2 +
 sysdeps/unix/sysv/linux/rseq-internal.h       | 91 +++++++++++++++++++
 sysdeps/unix/sysv/linux/rseq-sym.c            | 54 +++++++++++
 .../unix/sysv/linux/s390/s390-32/libc.abilist |  2 +
 .../unix/sysv/linux/s390/s390-64/libc.abilist |  2 +
 sysdeps/unix/sysv/linux/sh/libc.abilist       |  2 +
 .../sysv/linux/sparc/sparc32/libc.abilist     |  2 +
 .../sysv/linux/sparc/sparc64/libc.abilist     |  2 +
 sysdeps/unix/sysv/linux/sys/rseq.h            | 64 +++++++++++++
 .../unix/sysv/linux/x86_64/64/libc.abilist    |  2 +
 .../unix/sysv/linux/x86_64/x32/libc.abilist   |  2 +
 36 files changed, 328 insertions(+), 6 deletions(-)
 create mode 100644 misc/rseq-internal.h
 create mode 100644 sysdeps/unix/sysv/linux/rseq-internal.h
 create mode 100644 sysdeps/unix/sysv/linux/rseq-sym.c
 create mode 100644 sysdeps/unix/sysv/linux/sys/rseq.h
  

----- On Jan 21, 2019, at 4:35 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
[...]

> diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h
> b/sysdeps/unix/sysv/linux/sys/rseq.h
> new file mode 100644
> index 0000000000..61937fb193
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/sys/rseq.h
> @@ -0,0 +1,64 @@

[...]
> +
> +#ifndef _SYS_RSEQ_H
> +#define _SYS_RSEQ_H	1
> +
> +/* We use the structures declarations from the kernel headers.  */
> +#include <linux/rseq.h>
> +#include <stdint.h>
> +
> +/* Signature required before each abort handler code.  */
> +#define RSEQ_SIG 0x53053053

I recalled that aarch64 defines RSEQ_SIG to a different value which maps to
a valid trap instruction. So I plan to move the RSEQ_SIG define to per-arch
headers like this:

 sysdeps/unix/sysv/linux/aarch64/bits/rseq.h                  |   24 ++
 sysdeps/unix/sysv/linux/arm/bits/rseq.h                      |   24 ++
 sysdeps/unix/sysv/linux/bits/rseq.h                          |   23 ++
 sysdeps/unix/sysv/linux/mips/bits/rseq.h                     |   24 ++
 sysdeps/unix/sysv/linux/powerpc/bits/rseq.h                  |   24 ++
 sysdeps/unix/sysv/linux/s390/bits/rseq.h                     |   24 ++
 sysdeps/unix/sysv/linux/x86/bits/rseq.h                      |   24 ++

where "bits/rseq.h" contains a #error:

# error "Architecture does not define RSEQ_SIG.

sys/rseq.h will now include <bits/rseq.h>.


> +
> +enum rseq_register_state
> +{
> +  /* Value RSEQ_REGISTER_ALLOWED means it is allowed to update
> +     the refcount field and to register/unregister rseq.  */
> +  RSEQ_REGISTER_ALLOWED = 0,
> +  /* Value RSEQ_REGISTER_NESTED means it is temporarily forbidden
> +     to update the refcount field or to register/unregister rseq.  */
> +  RSEQ_REGISTER_NESTED = 1,

I plan to rename "RSEQ_REGISTER_NESTED" to "RSEQ_REGISTER_ONGOING" which
seems to better represent the current registration state.

Please let me know if anything is wrong with those changes.

Thanks,

Mathieu


> +  /* Value RSEQ_REGISTER_EXITING means it is forbidden to update the
> +     refcount field or to register/unregister rseq for the rest of the
> +     thread's lifetime.  */
> +  RSEQ_REGISTER_EXITING = 2,
> +};
  

On Tue, 29 Jan 2019, Mathieu Desnoyers wrote:

> I recalled that aarch64 defines RSEQ_SIG to a different value which maps to
> a valid trap instruction. So I plan to move the RSEQ_SIG define to per-arch
> headers like this:
> 
>  sysdeps/unix/sysv/linux/aarch64/bits/rseq.h                  |   24 ++
>  sysdeps/unix/sysv/linux/arm/bits/rseq.h                      |   24 ++
>  sysdeps/unix/sysv/linux/bits/rseq.h                          |   23 ++
>  sysdeps/unix/sysv/linux/mips/bits/rseq.h                     |   24 ++
>  sysdeps/unix/sysv/linux/powerpc/bits/rseq.h                  |   24 ++
>  sysdeps/unix/sysv/linux/s390/bits/rseq.h                     |   24 ++
>  sysdeps/unix/sysv/linux/x86/bits/rseq.h                      |   24 ++
> 
> where "bits/rseq.h" contains a #error:
> 
> # error "Architecture does not define RSEQ_SIG.
> 
> sys/rseq.h will now include <bits/rseq.h>.

We're trying to reduce the number of cases where most or all new glibc 
architecture ports need to provide a bits/ header, by making the generic 
headers handle the common case.  So a generic header with a #error, and 
lots of architecture-specific headers mostly with the same value for 
RSEQ_SIG, seems unfortunate.  I'd hope the generic header could use a 
generic value, with architecture-specific variants only for architectures 
with some reason for a different value.
  

----- On Jan 29, 2019, at 4:56 PM, Joseph Myers joseph@codesourcery.com wrote:

> On Tue, 29 Jan 2019, Mathieu Desnoyers wrote:
> 
>> I recalled that aarch64 defines RSEQ_SIG to a different value which maps to
>> a valid trap instruction. So I plan to move the RSEQ_SIG define to per-arch
>> headers like this:
>> 
>>  sysdeps/unix/sysv/linux/aarch64/bits/rseq.h                  |   24 ++
>>  sysdeps/unix/sysv/linux/arm/bits/rseq.h                      |   24 ++
>>  sysdeps/unix/sysv/linux/bits/rseq.h                          |   23 ++
>>  sysdeps/unix/sysv/linux/mips/bits/rseq.h                     |   24 ++
>>  sysdeps/unix/sysv/linux/powerpc/bits/rseq.h                  |   24 ++
>>  sysdeps/unix/sysv/linux/s390/bits/rseq.h                     |   24 ++
>>  sysdeps/unix/sysv/linux/x86/bits/rseq.h                      |   24 ++
>> 
>> where "bits/rseq.h" contains a #error:
>> 
>> # error "Architecture does not define RSEQ_SIG.
>> 
>> sys/rseq.h will now include <bits/rseq.h>.
> 
> We're trying to reduce the number of cases where most or all new glibc
> architecture ports need to provide a bits/ header, by making the generic
> headers handle the common case.  So a generic header with a #error, and
> lots of architecture-specific headers mostly with the same value for
> RSEQ_SIG, seems unfortunate.  I'd hope the generic header could use a
> generic value, with architecture-specific variants only for architectures
> with some reason for a different value.

The issue here is that it would require us to decide right away what RSEQ_SIG
is appropriate for all other Linux architectures supported by glibc. There are
a few reasons for which an architecture can be required to specify its own
RSEQ_SIG. For instance, it may need to map to an instruction defined in the
instruction set, thus ensuring objdump does not get confused, and in other
cases that the processor speculative execution happening just before the
RSEQ_SIG really stops at the signature (hence the trap instruction on aarch64).

I'm worried that if we introduce a "default" RSEQ_SIG value for architectures
currently not supported by RSEQ and we then introduce an architecture-specific
signature value in the future, some applications will try to build with
wrong signatures, and when the rseq system call gets eventually implemented for
those architecture and a end-user upgrades his kernel, those signatures won't
match between glibc rseq registration and the application rseq abort handlers,
thus leading to hard-to-reproduce segmentation faults delivered by the kernel
checking those signatures upon rseq abort.

This upgrade story is far from ideal.

My thinking was to put the #error in the generic header, so architectures that
are not supported yet cannot build against rseq.h at all, so we don't end up
in a broken upgrade scenario. I'm open to alternative ways to do it though, as
long as we don't let not-yet-supported architectures build broken code.

Thoughts ?

Thanks,

Mathieu
  

On Tue, 29 Jan 2019, Mathieu Desnoyers wrote:

> My thinking was to put the #error in the generic header, so architectures that
> are not supported yet cannot build against rseq.h at all, so we don't end up
> in a broken upgrade scenario. I'm open to alternative ways to do it though, as
> long as we don't let not-yet-supported architectures build broken code.

Any case with #error in installed glibc headers needs special-casing in 
check-installed-headers.sh (and, thus, such errors are to be discouraged).

Cases where architectures commonly need their own bits/ headers, 
especially where those are likely to need updating for new kernel 
versions, are also discouraged.  Furthermore, a normal check for glibc 
headers updates needed for a new kernel version would only involve 
examining uapi headers (and the non-uapi linux/socket.h for new address 
families, an unfortunate existing wart in this area).  As far as I can 
see, this value isn't defined in any uapi header, which makes it 
especially likely to be missed in such a check.  Furthermore, I'm hoping 
to add more glibc tests for consistency of such constants between glibc 
and the kernel, to ensure any such updates missing are caught 
automatically through test failures - but that doesn't work if the 
constants in question aren't in a uapi header.

If this constant were in a uapi header, the glibc header could just 
include that - is the issue that it's not actually an interface between 
glibc and the kernel at all, but some kind of purely-userspace interface?

We very definitely wish to keep to a minimum the cases where updates need 
to be done separately in glibc by each architecture maintainer (that's 
just a recipe for some updates getting missed accidentally) - meaning that 
there needs to be a clear way in which someone can tell, globally for all 
architectures, whether the set of such architecture-specific headers for 
this constant in glibc is complete and current, and when it needs updating 
(and this should be as similar to possible to such checks for any other 
header constant).
  

----- On Jan 29, 2019, at 9:40 PM, Joseph Myers joseph@codesourcery.com wrote:

> On Tue, 29 Jan 2019, Mathieu Desnoyers wrote:
> 
>> My thinking was to put the #error in the generic header, so architectures that
>> are not supported yet cannot build against rseq.h at all, so we don't end up
>> in a broken upgrade scenario. I'm open to alternative ways to do it though, as
>> long as we don't let not-yet-supported architectures build broken code.
> 
> Any case with #error in installed glibc headers needs special-casing in
> check-installed-headers.sh (and, thus, such errors are to be discouraged).

One alternative to #error would be to have an empty generic bits/rseq.h
that does _not_ define RSEQ_SIG. This way, it would be possible to
include sys/rseq.h from an architecture that does not define RSEQ_SIG
yet, but it would not cause any build failure. It's only if the code
try to use RSEQ_SIG that it would fail to compile because undefined.

> Cases where architectures commonly need their own bits/ headers,
> especially where those are likely to need updating for new kernel
> versions, are also discouraged.

The per-arch bits/rseq.h headers, once they define a specific value for
RSEQ_SIG, should never ever change that value.

> Furthermore, a normal check for glibc
> headers updates needed for a new kernel version would only involve
> examining uapi headers (and the non-uapi linux/socket.h for new address
> families, an unfortunate existing wart in this area).  As far as I can
> see, this value isn't defined in any uapi header, which makes it
> especially likely to be missed in such a check.  Furthermore, I'm hoping
> to add more glibc tests for consistency of such constants between glibc
> and the kernel, to ensure any such updates missing are caught
> automatically through test failures - but that doesn't work if the
> constants in question aren't in a uapi header.
> 
> If this constant were in a uapi header, the glibc header could just
> include that - is the issue that it's not actually an interface between
> glibc and the kernel at all, but some kind of purely-userspace interface?

The rseq uapi headers do not enforce the value of RSEQ_SIG. The role of the
kernel wrt signature is to receive it as sys_rseq argument, and then validate
that abort targets are prefixed with the signature before moving the
instruction pointer there.

Therefore, it's up to user-space to agree on the RSEQ_SIG value across
all code using rseq within a process. Since glibc will be registering
rseq and exposing public headers, it appears that glibc would be the
appropriate project to define the RSEQ_SIG value for each architecture.

> 
> We very definitely wish to keep to a minimum the cases where updates need
> to be done separately in glibc by each architecture maintainer (that's
> just a recipe for some updates getting missed accidentally) - meaning that
> there needs to be a clear way in which someone can tell, globally for all
> architectures, whether the set of such architecture-specific headers for
> this constant in glibc is complete and current, and when it needs updating
> (and this should be as similar to possible to such checks for any other
> header constant).

Currently, I use #ifdef __NR_rseq from uapi unistd.h to check whether the
kernel headers implement the rseq system call for the target architecture.

With the approach of having an empty bits/rseq.h for architecture not yet
supporting rseq in glibc, one way to check that glibc implements RSEQ_SIG
for all architectures that have the rseq system call wired up in uapi would
be:

#include <unistd.h>
#include <sys/rseq.h>

#if defined (__NR_rseq) && !defined (RSEQ_SIG)
# error "UAPI headers support rseq system call, but glibc does not define RSEQ_SIG."
#endif

Would that take care of your concerns ?

Thanks,

Mathieu
  

On Wed, Jan 30, 2019 at 1:03 PM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> ----- On Jan 29, 2019, at 9:40 PM, Joseph Myers joseph@codesourcery.com wrote:
> > On Tue, 29 Jan 2019, Mathieu Desnoyers wrote:
> >
> >> My thinking was to put the #error in the generic header, so architectures that
> >> are not supported yet cannot build against rseq.h at all, so we don't end up
> >> in a broken upgrade scenario. I'm open to alternative ways to do it though, as
> >> long as we don't let not-yet-supported architectures build broken code.
> >
> > Any case with #error in installed glibc headers needs special-casing in
> > check-installed-headers.sh (and, thus, such errors are to be discouraged).
>
> One alternative to #error would be to have an empty generic bits/rseq.h
> that does _not_ define RSEQ_SIG. This way, it would be possible to
> include sys/rseq.h from an architecture that does not define RSEQ_SIG
> yet, but it would not cause any build failure. It's only if the code
> try to use RSEQ_SIG that it would fail to compile because undefined.

You seem to be clinging to an approach where every architecture (that
does support rseq(2), which is hopefully going to be all of them in
the near future) has to define its own bits/rseq.h.  That's exactly
the thing we don't want.

Could you please explain why you believe it is more important to have
build errors, in the short term, on architectures that don't support
rseq(2) yet, than to improve the maintainability of the code in the
long term?

> > If this constant were in a uapi header, the glibc header could just
> > include that - is the issue that it's not actually an interface between
> > glibc and the kernel at all, but some kind of purely-userspace interface?
>
> The rseq uapi headers do not enforce the value of RSEQ_SIG. The role of the
> kernel wrt signature is to receive it as sys_rseq argument, and then validate
> that abort targets are prefixed with the signature before moving the
> instruction pointer there.

In that case, is there any reason not to use the same value on _all_
architectures?  Or maybe the same value on all 32-bit architectures,
and another one on all 64-bit architectures?

(Based only on what you just said, I can imagine a reason: if this
"signature" is baked into the code segments of programs, then it may
need to be chosen for each architecture so it doesn't collide with the
encoding of any valid machine instruction.  But then it would be fixed
as part of the ABI and the kernel should know what it is, rather than
having to be told during process startup.  It's quite possible I have
misunderstood.  I have not been following this discussion closely.)

zw
  

* Zack Weinberg:

> In that case, is there any reason not to use the same value on _all_
> architectures?  Or maybe the same value on all 32-bit architectures,
> and another one on all 64-bit architectures?

I think the intent here is to use a value that would be extremely
unlikely to appear in the instruction stream, so that jump target
pointer in struct rseq_cs can be somewhat validated before the program
counter is updated.

This doesn't have to be a trapping instruction (in fact, the default
trapping instruction would probably be a bad choice because the
instruction does appear in the text segment quite frequently).  But even
if we choose an optimal value for all currently supported architectures,
we might add an architecture in the future which prefers a different
value.

Thanks,
Florian
  

----- On Jan 30, 2019, at 1:30 PM, Zack Weinberg zackw@panix.com wrote:

> On Wed, Jan 30, 2019 at 1:03 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>> ----- On Jan 29, 2019, at 9:40 PM, Joseph Myers joseph@codesourcery.com wrote:
>> > On Tue, 29 Jan 2019, Mathieu Desnoyers wrote:
>> >
>> >> My thinking was to put the #error in the generic header, so architectures that
>> >> are not supported yet cannot build against rseq.h at all, so we don't end up
>> >> in a broken upgrade scenario. I'm open to alternative ways to do it though, as
>> >> long as we don't let not-yet-supported architectures build broken code.
>> >
>> > Any case with #error in installed glibc headers needs special-casing in
>> > check-installed-headers.sh (and, thus, such errors are to be discouraged).
>>
>> One alternative to #error would be to have an empty generic bits/rseq.h
>> that does _not_ define RSEQ_SIG. This way, it would be possible to
>> include sys/rseq.h from an architecture that does not define RSEQ_SIG
>> yet, but it would not cause any build failure. It's only if the code
>> try to use RSEQ_SIG that it would fail to compile because undefined.
> 
> You seem to be clinging to an approach where every architecture (that
> does support rseq(2), which is hopefully going to be all of them in
> the near future) has to define its own bits/rseq.h.  That's exactly
> the thing we don't want.
>
> Could you please explain why you believe it is more important to have
> build errors, in the short term, on architectures that don't support
> rseq(2) yet, than to improve the maintainability of the code in the
> long term?

I'm open to alternative solutions, provided that they don't lead to
glibc/kernel headers/kernel image upgrade scenarios that trigger
segmentation faults.

I just lack imagination in my search to find a solution that fulfills
the non-segfault-on-upgrade requirement without the burden of a
per-architecture bits/rseq.h to define RSEQ_SIG. If we find a solution
that fulfills both goals, I'm all for it.

Note that the alternative solution I hint at above does not use #error,
but rather just leaves RSEQ_SIG undefined for architectures that don't
support rseq yet. It still requires to have per-architecture bits/rseq.h,
which appears to be a maintainability burden though.

> 
>> > If this constant were in a uapi header, the glibc header could just
>> > include that - is the issue that it's not actually an interface between
>> > glibc and the kernel at all, but some kind of purely-userspace interface?
>>
>> The rseq uapi headers do not enforce the value of RSEQ_SIG. The role of the
>> kernel wrt signature is to receive it as sys_rseq argument, and then validate
>> that abort targets are prefixed with the signature before moving the
>> instruction pointer there.
> 
> In that case, is there any reason not to use the same value on _all_
> architectures?  Or maybe the same value on all 32-bit architectures,
> and another one on all 64-bit architectures?

As I stated earlier in the thread, we've encountered a few reasons that
justify overriding the RSEQ_SIG for specific architectures so far:

- Playing nice with objdump quirks by ensuring the signature is a
  valid instruction,
- Ensuring the architecture's speculative execution stops at the signature
  by making sure this is a specific trap instruction that is not found
  typically elsewhere in the code,
- Ensuring the chosen RSEQ_SIG does not happen to be a common byte sequence
  frequently present in the code segments of each target architecture.

> 
> (Based only on what you just said, I can imagine a reason: if this
> "signature" is baked into the code segments of programs, then it may
> need to be chosen for each architecture so it doesn't collide with the
> encoding of any valid machine instruction.  But then it would be fixed
> as part of the ABI and the kernel should know what it is, rather than
> having to be told during process startup.  It's quite possible I have
> misunderstood.  I have not been following this discussion closely.)

As you figured out, this signature needs to be baked into the application
and library code. However, the way rseq uapi is defined, the choice of
the signature is left to user-space. It is not defined by the kernel uapi.

The kernel just validates that signature, which is received as input
parameter to the rseq system call, and needs to be found before abort
target labels.

The main reason why we left the signature choice to user-space is that
we may have to eventually turn the "fixed" signature into a randomly
assigned one for hardened processes. We have to recall that the purpose
of this signature is to protect against control flow redirection by an
attacker through the rseq abort mechanism, so I prefer not to fix it
to a static value within the kernel uapi, and leave that decision to
user-space, which would allow randomizing it on process startup for
hardened processes in the future.

Thanks,

Mathieu

> 
> zw
  

On Wed, 30 Jan 2019, Mathieu Desnoyers wrote:

> #if defined (__NR_rseq) && !defined (RSEQ_SIG)
> # error "UAPI headers support rseq system call, but glibc does not define RSEQ_SIG."
> #endif
> 
> Would that take care of your concerns ?

That would of course need appropriate conditionals based on the most 
recent kernel version for which a given glibc version has been updated, so 
that using new kernel headers with an existing glibc release does not make 
the build fail (cf. the test of syscall-names.list).  And being able to 
write such a test only solves one half of the problem - it needs to be 
easy to determine what value to put in that header in glibc for an 
architecture that's newly gained support in the kernel, *without* needing 
any architecture expertise.
  

----- On Jan 30, 2019, at 4:10 PM, Joseph Myers joseph@codesourcery.com wrote:

> On Wed, 30 Jan 2019, Mathieu Desnoyers wrote:
> 
>> #if defined (__NR_rseq) && !defined (RSEQ_SIG)
>> # error "UAPI headers support rseq system call, but glibc does not define
>> RSEQ_SIG."
>> #endif
>> 
>> Would that take care of your concerns ?
> 
> That would of course need appropriate conditionals based on the most
> recent kernel version for which a given glibc version has been updated, so
> that using new kernel headers with an existing glibc release does not make
> the build fail (cf. the test of syscall-names.list).

The test I hint at above would not be for the glibc build per se. It would
be for a check that glibc implements support for all the system calls
available in the kernel headers (if such a test target currently exists).

> And being able to
> write such a test only solves one half of the problem - it needs to be
> easy to determine what value to put in that header in glibc for an
> architecture that's newly gained support in the kernel, *without* needing
> any architecture expertise.

I'm afraid this requirement is incompatible with the nature of the RSEQ
signature. This signature may be required to be a specific trap instruction
by the architecture, so deciding on its value without architecture expertise
is not possible.

Thanks,

Mathieu
  

----- On Jan 31, 2019, at 11:37 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> ----- On Jan 30, 2019, at 4:10 PM, Joseph Myers joseph@codesourcery.com wrote:
> 
>> On Wed, 30 Jan 2019, Mathieu Desnoyers wrote:
>> 
>>> #if defined (__NR_rseq) && !defined (RSEQ_SIG)
>>> # error "UAPI headers support rseq system call, but glibc does not define
>>> RSEQ_SIG."
>>> #endif
>>> 
>>> Would that take care of your concerns ?
>> 
>> That would of course need appropriate conditionals based on the most
>> recent kernel version for which a given glibc version has been updated, so
>> that using new kernel headers with an existing glibc release does not make
>> the build fail (cf. the test of syscall-names.list).
> 
> The test I hint at above would not be for the glibc build per se. It would
> be for a check that glibc implements support for all the system calls
> available in the kernel headers (if such a test target currently exists).
> 
>> And being able to
>> write such a test only solves one half of the problem - it needs to be
>> easy to determine what value to put in that header in glibc for an
>> architecture that's newly gained support in the kernel, *without* needing
>> any architecture expertise.
> 
> I'm afraid this requirement is incompatible with the nature of the RSEQ
> signature. This signature may be required to be a specific trap instruction
> by the architecture, so deciding on its value without architecture expertise
> is not possible.

Just to clarify a point: the "success criterion" I'm aiming for here is to
provide a rseq integration that does not cause foreseeable user crashes on
upgrade.

I'm all for taking into account the maintenance burden on glibc maintainers as
a metric in the implementation choices made, but at this point, I don't see
how we can achieve success without introducing architecture headers for the
RSEQ_SIG signature. If you have ideas on how to further minimize the maintenance
burden for glibc maintainers while still meeting the success criterion, I'm all
ears.

Thanks,

Mathieu