public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Joseph Myers <joseph@codesourcery.com>
Cc: carlos <carlos@redhat.com>, Florian Weimer <fweimer@redhat.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	libc-alpha <libc-alpha@sourceware.org>,
	Thomas Gleixner <tglx@linutronix.de>, Ben Maurer <bmaurer@fb.com>,
	Peter Zijlstra <peterz@infradead.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Will Deacon <will.deacon@arm.com>,
	Dave Watson <davejwatson@fb.com>, Paul Turner <pjt@google.com>,
	Rich Felker <dalias@libc.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-api <linux-api@vger.kernel.org>
Subject: Re: [RFC PATCH glibc 1/4] glibc: Perform rseq(2) registration at C startup and thread creation (v6)
Date: Wed, 30 Jan 2019 13:03:46 -0500 (EST)	[thread overview]
Message-ID: <1832200535.4162.1548871426959.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1901300231210.24454@digraph.polyomino.org.uk>

----- 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

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

  reply	other threads:[~2019-01-30 18:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-21 21:35 [RFC PATCH glibc 1/4] glibc: Perform rseq(2) registration at C startup and thread creation (v6) Mathieu Desnoyers
2019-01-21 21:35 ` [RFC PATCH glibc 2/4] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux Mathieu Desnoyers
2019-01-29 16:57 ` [RFC PATCH glibc 1/4] glibc: Perform rseq(2) registration at C startup and thread creation (v6) Mathieu Desnoyers
2019-01-29 21:56   ` Joseph Myers
2019-01-30  1:39     ` Mathieu Desnoyers
2019-01-30  2:40       ` Joseph Myers
2019-01-30 18:03         ` Mathieu Desnoyers [this message]
2019-01-30 21:10           ` Joseph Myers
2019-01-31 16:37             ` Mathieu Desnoyers
2019-01-31 16:53               ` Mathieu Desnoyers

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=1832200535.4162.1548871426959.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=bmaurer@fb.com \
    --cc=boqun.feng@gmail.com \
    --cc=carlos@redhat.com \
    --cc=dalias@libc.org \
    --cc=davejwatson@fb.com \
    --cc=fweimer@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=szabolcs.nagy@arm.com \
    --cc=tglx@linutronix.de \
    --cc=will.deacon@arm.com \
    /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