linux-kernel - Re: BUG: rseq selftests and librseq vs. glibc fail

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aKODByTQMYFs3WVN@google.com>
Date: Mon, 18 Aug 2025 12:46:15 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Florian Weimer <fweimer@...hat.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Jens Axboe <axboe@...nel.dk>, 
	LKML <linux-kernel@...r.kernel.org>, Michael Jeanson <mjeanson@...icios.com>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Peter Zijlstra <peterz@...radead.org>, 
	"Paul E. McKenney" <paulmck@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Wei Liu <wei.liu@...nel.org>, 
	Samuel Thibault <sthibault@...ian.org>
Subject: Re: BUG: rseq selftests and librseq vs. glibc fail

On Mon, Aug 18, 2025, Florian Weimer wrote:
> * Thomas Gleixner:
> 
> > On Mon, Aug 18 2025 at 16:15, Florian Weimer wrote:
> >> * Thomas Gleixner:
> >>> It's trivial to reproduce. All it needs is to have in the source:
> >>>
> >>> __weak ptrdiff_t __rseq_offset;
> >>>
> >>> w/o even being referenced and creating a pthread. Reproducer below.
> >>
> >> Well, that's sort of expected.  You can't define glibc symbols that are
> >> not intended for interposition and expect things to work.  It's kind of
> >> like writing:
> >>
> >> int _rtld_global;
> >>
> >> That's going to fail rather spectaculary, too.  We make an exception for
> >> symbols that are not reserved (you can build in ISO C mode and define
> >> open, close, etc., at least as long as you link to glibc only).  But
> >> __rseq_offset is a reserved name, so that is not applicable here.
> >>
> >> The real change here is GCC changing from -fcommon (which made a lot of
> >> these things work in the past) to -fno-common.
> >
> > Thanks for the explanation!
> >
> > So the only way to make this actually work is to revert that commit and
> > the folks who want to link that statically need to come up with:
> >
> > #ifdef _BUILD_STATICALLY
> > extern ....
> >
> > #else
> >         ptr = dlsym(...);
> > #endif
> >
> > or something daft like that. A proper function interface would avoid all
> > that nonsense, but we can't have nice things or can we?
> 
> I don't understand why a function would be different.  Well, a function
> *declaration* would be implicitly extern, in a way a variable
> declaration is not (without -fcommon).  Maybe it's just about the
> missing extern keyword?
> 
> You could add the extern keyword and check &__rseq_offset for NULL if
> you want to probe for the availability of the signal?

That will fail to link if the glibc version doesn't support rseq in any capacity,
which is why I added the __weak crud.

>Or use:
> 
> #if __has_include(<sys/rseq.h>)
> #include <sys/rseq.h>
> /* Code that depends on glibc's rseq support goes here. */

FWIW, the code in question doesn't depend on rseq per se, rather the problem is
that attempting to register a restartable sequence fails if glibc has already
"claimed" rseq.

What about something horrific like this?  Or if __has_include(<sys/rseq.h>) is
preferrable to checking the glibc version, go with that.  The idea with checking
LIBC_RSEQ_STATIC_LINK is to give folks a way to force static linking if their
libc registers rseq, but doesn't satisfy the glibc version checks for whatever
reason.

diff --git a/tools/testing/selftests/rseq/rseq.c b/tools/testing/selftests/rseq/rseq.c
index 663a9cef1952..1a88352fcff3 100644
--- a/tools/testing/selftests/rseq/rseq.c
+++ b/tools/testing/selftests/rseq/rseq.c
@@ -36,17 +36,18 @@
 #include "../kselftest.h"
 #include "rseq.h"
 
-/*
- * Define weak versions to play nice with binaries that are statically linked
- * against a libc that doesn't support registering its own rseq.
- */
-__weak ptrdiff_t __rseq_offset;
-__weak unsigned int __rseq_size;
-__weak unsigned int __rseq_flags;
+#if defined(LIBC_RSEQ_STATIC_LINK) || __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 35)
+extern ptrdiff_t __rseq_offset;
+extern unsigned int __rseq_size;
+extern unsigned int __rseq_flags;
+#define GLIBC_RSEQ_PTR(x) &x
+#else
+#define GLIBC_RSEQ_PTR(x) NULL
+#endif
 
-static const ptrdiff_t *libc_rseq_offset_p = &__rseq_offset;
-static const unsigned int *libc_rseq_size_p = &__rseq_size;
-static const unsigned int *libc_rseq_flags_p = &__rseq_flags;
+static const ptrdiff_t *libc_rseq_offset_p = GLIBC_RSEQ_PTR(__rseq_offset);
+static const unsigned int *libc_rseq_size_p = GLIBC_RSEQ_PTR(__rseq_size);
+static const unsigned int *libc_rseq_flags_p = GLIBC_RSEQ_PTR(__rseq_flags);
 
 /* Offset from the thread pointer to the rseq area. */
 ptrdiff_t rseq_offset;
@@ -209,7 +210,7 @@ void rseq_init(void)
         * libc not having registered a restartable sequence.  Try to find the
         * symbols if that's the case.
         */
-       if (!*libc_rseq_size_p) {
+       if (!libc_rseq_size_p || !*libc_rseq_size_p) {
                libc_rseq_offset_p = dlsym(RTLD_NEXT, "__rseq_offset");
                libc_rseq_size_p = dlsym(RTLD_NEXT, "__rseq_size");
                libc_rseq_flags_p = dlsym(RTLD_NEXT, "__rseq_flags");