Add "extern" to the glibc-defined weak rseq symbols to convert the rseq selftest's usage from weak symbol definitions to weak symbol _references_. Effectively re-defining the glibc symbols wreaks havoc when building with -fno-common, e.g. generates segfaults when running multi-threaded programs, as dynamically linked applications end up with multiple versions of the symbols.
Building with -fcommon, which until recently has the been the default for GCC and clang, papers over the bug by allowing the linker to resolve the weak/tentative definition to glibc's "real" definition.
Note, the symbol itself (or rather its address), not the value of the symbol, is set to 0/NULL for unresolved weak symbol references, as the symbol doesn't exist and thus can't have a value. Check for a NULL rseq size pointer to handle the scenario where the test is statically linked against a libc that doesn't support rseq in any capacity.
Fixes: 3bcbc20942db ("selftests/rseq: Play nice with binaries statically linked against glibc 2.35+") Cc: stable@vger.kernel.org Suggested-by: Florian Weimer fweimer@redhat.com Reported-by: Thomas Gleixner tglx@linutronix.de Closes: https://lore.kernel.org/all/87frdoybk4.ffs@tglx Signed-off-by: Sean Christopherson seanjc@google.com --- tools/testing/selftests/rseq/rseq.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/rseq/rseq.c b/tools/testing/selftests/rseq/rseq.c index 663a9cef1952..dcac5cbe7933 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_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");
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
On 2025-08-19 18:29, Sean Christopherson wrote:
Add "extern" to the glibc-defined weak rseq symbols to convert the rseq selftest's usage from weak symbol definitions to weak symbol _references_. Effectively re-defining the glibc symbols wreaks havoc when building with -fno-common, e.g. generates segfaults when running multi-threaded programs, as dynamically linked applications end up with multiple versions of the symbols.
Building with -fcommon, which until recently has the been the default for GCC and clang, papers over the bug by allowing the linker to resolve the weak/tentative definition to glibc's "real" definition.
Note, the symbol itself (or rather its address), not the value of the symbol, is set to 0/NULL for unresolved weak symbol references, as the symbol doesn't exist and thus can't have a value. Check for a NULL rseq size pointer to handle the scenario where the test is statically linked against a libc that doesn't support rseq in any capacity.
Fixes: 3bcbc20942db ("selftests/rseq: Play nice with binaries statically linked against glibc 2.35+") Cc: stable@vger.kernel.org Suggested-by: Florian Weimer fweimer@redhat.com Reported-by: Thomas Gleixner tglx@linutronix.de Closes: https://lore.kernel.org/all/87frdoybk4.ffs@tglx Signed-off-by: Sean Christopherson seanjc@google.com
+ CC: Michael Jeanson mjeanson@efficios.com
tools/testing/selftests/rseq/rseq.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/rseq/rseq.c b/tools/testing/selftests/rseq/rseq.c index 663a9cef1952..dcac5cbe7933 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.
We should therefore update the comment above to e.g.:
/* * Define weak symbol references to play nice with binaries that are * statically linked against a libc that doesn't support registering its * own rseq. */
For the rest:
Reviewed-by: Mathieu Desnoyers mathieu.desnoyers@efficios.com
Michael, can you try it out ?
Kienan, we may want to add a configuration forcing "-fno-common" to our CI, this problematic pattern may be hiding other issues elsewhere. I'm thinking of LTTng-UST tracepoint headers and libside headers.
Sean, do you want to contribute the fix to librseq as well ?
Thanks everyone for looking into this. I'll be back from vacation next week and will resume normal operations. :-)
Mathieu
*/ -__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_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");
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
On 2025-08-20 08:55, Mathieu Desnoyers wrote:
Michael, can you try it out ?
Will do.
Kienan, we may want to add a configuration forcing "-fno-common" to our CI, this problematic pattern may be hiding other issues elsewhere. I'm thinking of LTTng-UST tracepoint headers and libside headers.
AFAIK, -fno-common has been the default since GCC 10 so it's already well tested in CI, there are a few fixes from around 2020 in LTTng related to this.
Sean, do you want to contribute the fix to librseq as well ?
Librseq currently doesn't have the weak symbols like the selftests so the fix doesn't really apply.
Thanks everyone for looking into this. I'll be back from vacation next week and will resume normal operations. :-)
Mathieu
*/
-__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_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");
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
linux-kselftest-mirror@lists.linaro.org