All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Florian Weimer <fweimer@redhat.com>, Jens Axboe <axboe@kernel.dk>,
	 LKML <linux-kernel@vger.kernel.org>,
	Michael Jeanson <mjeanson@efficios.com>,
	 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Peter Zijlstra <peterz@infradead.org>,
	 "Paul E. McKenney" <paulmck@kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>, Wei Liu <wei.liu@kernel.org>,
	 Samuel Thibault <sthibault@debian.org>
Subject: Re: BUG: rseq selftests and librseq vs. glibc fail
Date: Mon, 18 Aug 2025 17:28:17 -0700	[thread overview]
Message-ID: <aKPFIQwg5zxSS5oS@google.com> (raw)
In-Reply-To: <87349oxk2n.ffs@tglx>

On Tue, Aug 19, 2025, Thomas Gleixner wrote:
> On Mon, Aug 18 2025 at 13:27, Sean Christopherson wrote:
> > On Mon, Aug 18, 2025, Florian Weimer wrote:
> >> You need both (extern and weak) to get a weak symbol reference instead
> >> of a weak symbol definition.  You still need to check &__rseq_offset, of
> >> course.
> >
> > Ooh, you're saying add "extern" to the existing __weak symbol, not replace it.
> > Huh, TIL weak symbol references are a thing.
> >
> > This works with static and dynamic linking, with and without an rseq-aware glibc.
> >
> > Thomas, does this fix the problem you were seeing?
> >
> > diff --git a/tools/testing/selftests/rseq/rseq.c b/tools/testing/selftests/rseq/rseq.c
> > index 663a9cef1952..d17ded120d48 100644
> > --- a/tools/testing/selftests/rseq/rseq.c
> > +++ b/tools/testing/selftests/rseq/rseq.c
> > @@ -40,9 +40,9 @@
> >   * 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;
> > +extern __weak ptrdiff_t __rseq_offset;
> > +extern __weak unsigned int __rseq_size;
> > +extern __weak unsigned int __rseq_flags;
> >  
> >  static const ptrdiff_t *libc_rseq_offset_p = &__rseq_offset;
> >  static const unsigned int *libc_rseq_size_p = &__rseq_size;
> > @@ -209,7 +209,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_offset_p || !*libc_rseq_size_p) {

Doh, I meant to check libc_rseq_size_p for NULL, i.e.

	if (!libc_rseq_size_p || !*libc_rseq_size_p) {

> 
> If I make that:
> 
> +       if (!*libc_rseq_offset_p || !*libc_rseq_size_p) {
> 
> then it makes sense and actually works. The pointer can hardly be NULL,
> even when statically linked, no?

IIUC, it is indeed the pointers that are set to NULL/0, because for unresolved
symbols, the symbol itself, not its value, is set to '0'.  Which makes sense,
because if there is no symbol, then it can't have a value.

I.e. the address of the symbol is '0', and its value is undefined.

E.g. statically linking this against glibc without rseq support:

diff --git a/tools/testing/selftests/rseq/rseq.c b/tools/testing/selftests/rseq/rseq.c
index 663a9cef1952..959bdcb32e96 100644
--- a/tools/testing/selftests/rseq/rseq.c
+++ b/tools/testing/selftests/rseq/rseq.c
@@ -40,9 +40,9 @@
  * 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;
+extern __weak ptrdiff_t __rseq_offset;
+extern __weak unsigned int __rseq_size;
+extern __weak unsigned int __rseq_flags;
 
 static const ptrdiff_t *libc_rseq_offset_p = &__rseq_offset;
 static const unsigned int *libc_rseq_size_p = &__rseq_size;
@@ -209,7 +209,12 @@ 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) {
+       printf("libc_rseq_offset_p = %lx (%lx), libc_rseq_size_p = %lx (%lx)\n",
+              (unsigned long)libc_rseq_offset_p, (unsigned long)libc_rseq_size_p,
+              (unsigned long)&__rseq_offset, (unsigned long)&__rseq_size);
+       printf("__rseq_size = %u\n", __rseq_size);
+
+       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");

Generates this output:

  $ ./rseq_test 
  libc_rseq_offset_p = 0 (0), libc_rseq_size_p = 0 (0)
  Segmentation fault

Because trying to dereference __rseq_size hits NULL/0.

  reply	other threads:[~2025-08-19  0:28 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-13 16:29 [patch 00/11] rseq: Optimize exit to user space Thomas Gleixner
2025-08-13 16:29 ` [patch 01/11] rseq: Avoid pointless evaluation in __rseq_notify_resume() Thomas Gleixner
2025-08-20 14:23   ` Mathieu Desnoyers
2025-08-13 16:29 ` [patch 02/11] rseq: Condense the inline stubs Thomas Gleixner
2025-08-20 14:24   ` Mathieu Desnoyers
2025-08-13 16:29 ` [patch 03/11] rseq: Rename rseq_syscall() to rseq_debug_syscall_exit() Thomas Gleixner
2025-08-20 14:25   ` Mathieu Desnoyers
2025-08-13 16:29 ` [patch 04/11] rseq: Replace the pointless event mask bit fiddling Thomas Gleixner
2025-08-13 16:29 ` [patch 05/11] rseq: Optimize the signal delivery path Thomas Gleixner
2025-08-13 16:29 ` [patch 06/11] rseq: Optimize exit to user space further Thomas Gleixner
2025-08-13 16:29 ` [patch 07/11] entry: Cleanup header Thomas Gleixner
2025-08-13 17:09   ` Giorgi Tchankvetadze
2025-08-13 21:30     ` Thomas Gleixner
2025-08-13 16:29 ` [patch 08/11] entry: Distinguish between syscall and interrupt exit Thomas Gleixner
2025-08-13 16:29 ` [patch 09/11] entry: Provide exit_to_user_notify_resume() Thomas Gleixner
2025-08-13 16:29 ` [patch 10/11] rseq: Skip fixup when returning from a syscall Thomas Gleixner
2025-08-14  8:54   ` Peter Zijlstra
2025-08-14 13:24     ` Thomas Gleixner
2025-08-13 16:29 ` [patch 11/11] rseq: Convert to masked user access where applicable Thomas Gleixner
2025-08-13 17:45 ` [patch 00/11] rseq: Optimize exit to user space Jens Axboe
2025-08-13 21:32   ` Thomas Gleixner
2025-08-13 21:36     ` Jens Axboe
2025-08-13 22:08       ` Thomas Gleixner
2025-08-17 21:23         ` Thomas Gleixner
2025-08-18 14:00           ` BUG: rseq selftests and librseq vs. glibc fail Thomas Gleixner
2025-08-18 14:15             ` Florian Weimer
2025-08-18 17:13               ` Thomas Gleixner
2025-08-18 19:33                 ` Florian Weimer
2025-08-18 19:46                   ` Sean Christopherson
2025-08-18 19:55                     ` Florian Weimer
2025-08-18 20:27                       ` Sean Christopherson
2025-08-18 23:54                         ` Thomas Gleixner
2025-08-19  0:28                           ` Sean Christopherson [this message]
2025-08-19  6:18                             ` Florian Weimer
2025-08-29 18:44                 ` Prakash Sangappa
2025-08-29 18:50                   ` Mathieu Desnoyers
2025-09-01 19:30                     ` Prakash Sangappa
2025-08-18 17:38           ` [patch 00/11] rseq: Optimize exit to user space Michael Jeanson
2025-08-18 20:21             ` Thomas Gleixner
2025-08-18 21:29               ` Michael Jeanson
2025-08-18 23:43                 ` Thomas Gleixner
2025-08-20 14:27           ` Mathieu Desnoyers
2025-08-20 14:10 ` 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=aKPFIQwg5zxSS5oS@google.com \
    --to=seanjc@google.com \
    --cc=axboe@kernel.dk \
    --cc=boqun.feng@gmail.com \
    --cc=fweimer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mjeanson@efficios.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=sthibault@debian.org \
    --cc=tglx@linutronix.de \
    --cc=wei.liu@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.