[v8,6/6] elf: Raise SIGABRT for assert within ld.so
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
The assert static path already properly raises SIGABRT for assert during
process startup, while ld.so only prints the error message and issues
_exit. This prevents a coredump from being created if the environment
is configured to do so.
Similar to rtld malloc hooks, the assert symbol __assert_fail and
assert_perror_fail need to be relocated to the libc symbol after the
final relocation (so dlfcn.h functions call libc __assert_fail instead
of dl-minimal.c one). The hook framework is renamed to be more generic,
instead of tying to rtld malloc.
The Hurd requires a proper __raise_direct implementation, issuing raise
or kill pull a lot of symbols that prevents ld.so building.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
elf/dl-minimal.c | 82 ++++++++++++++++++++++++--------
elf/rtld.c | 4 +-
include/assert.h | 2 +
include/rtld-assert.h | 55 +++++++++++++++++++++
include/rtld-malloc.h | 14 ------
include/stdio.h | 2 +
sysdeps/generic/ldsodefs.h | 16 +++++++
sysdeps/mach/hurd/Makefile | 6 +++
sysdeps/mach/hurd/raise_direct.c | 29 +++++++++++
9 files changed, 175 insertions(+), 35 deletions(-)
create mode 100644 include/rtld-assert.h
create mode 100644 sysdeps/mach/hurd/raise_direct.c
Comments
* Adhemerval Zanella:
> The assert static path already properly raises SIGABRT for assert during
> process startup, while ld.so only prints the error message and issues
> _exit. This prevents a coredump from being created if the environment
> is configured to do so.
>
> Similar to rtld malloc hooks, the assert symbol __assert_fail and
> assert_perror_fail need to be relocated to the libc symbol after the
> final relocation (so dlfcn.h functions call libc __assert_fail instead
> of dl-minimal.c one). The hook framework is renamed to be more generic,
> instead of tying to rtld malloc.
>
> The Hurd requires a proper __raise_direct implementation, issuing raise
> or kill pull a lot of symbols that prevents ld.so building.
>
> Checked on x86_64-linux-gnu and i686-linux-gnu.
Is it really necessary to provide an interceptable assert for ld.so
assertion failures? We could reset the handler unconditionally and then
send SIGABRT (in a loop, due to race conditions).
With this approach, we wouldn't need stubs that get later replaced with
the libc implementation.
I don't think providing SIGABRT interception capabilities is woth the
complexity.
Thanks,
Florian
On 20/03/26 08:12, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> The assert static path already properly raises SIGABRT for assert during
>> process startup, while ld.so only prints the error message and issues
>> _exit. This prevents a coredump from being created if the environment
>> is configured to do so.
>>
>> Similar to rtld malloc hooks, the assert symbol __assert_fail and
>> assert_perror_fail need to be relocated to the libc symbol after the
>> final relocation (so dlfcn.h functions call libc __assert_fail instead
>> of dl-minimal.c one). The hook framework is renamed to be more generic,
>> instead of tying to rtld malloc.
>>
>> The Hurd requires a proper __raise_direct implementation, issuing raise
>> or kill pull a lot of symbols that prevents ld.so building.
>>
>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>
> Is it really necessary to provide an interceptable assert for ld.so
> assertion failures? We could reset the handler unconditionally and then
> send SIGABRT (in a loop, due to race conditions).
It allows a more clear definitions of the requirements, the assert
during process execution requires just 'raise (SIGABRT)' (I added the
ABORT_INSTRUCTION and _exit for completeness, there are not strictly
required).
We changed the libc abort to avoid clever solutions to handle the
potential race condition, I think we avoid adding some similar.
>
> With this approach, we wouldn't need stubs that get later replaced with
> the libc implementation.
>
> I don't think providing SIGABRT interception capabilities is woth the
> complexity.
>
> Thanks,
> Florian
>
* Adhemerval Zanella:
> @@ -188,11 +232,11 @@ __assert_perror_fail (int errnum,
> const char *function)
> {
> char errbuf[400];
> - _dl_fatal_printf ("\
> + _dl_error_printf ("\
> Inconsistency detected by ld.so: %s: %u: %s%sUnexpected error: %s.\n",
> file, line, function ?: "", function ? ": " : "",
> __strerror_r (errnum, errbuf, sizeof errbuf));
> -
> + rtld_abort ();
> }
Would it make sense to change _dl_fatal_printf to abort instead?
It requires changing the _dl_fatal_printf call in _dl_signal_error
because we don't want to abort if loading fails due to a missing
dependency.
Thanks,
Florian
* Florian Weimer:
> * Adhemerval Zanella:
>
>> The assert static path already properly raises SIGABRT for assert during
>> process startup, while ld.so only prints the error message and issues
>> _exit. This prevents a coredump from being created if the environment
>> is configured to do so.
>>
>> Similar to rtld malloc hooks, the assert symbol __assert_fail and
>> assert_perror_fail need to be relocated to the libc symbol after the
>> final relocation (so dlfcn.h functions call libc __assert_fail instead
>> of dl-minimal.c one). The hook framework is renamed to be more generic,
>> instead of tying to rtld malloc.
>>
>> The Hurd requires a proper __raise_direct implementation, issuing raise
>> or kill pull a lot of symbols that prevents ld.so building.
>>
>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>
> Is it really necessary to provide an interceptable assert for ld.so
> assertion failures? We could reset the handler unconditionally and then
> send SIGABRT (in a loop, due to race conditions).
>
> With this approach, we wouldn't need stubs that get later replaced with
> the libc implementation.
>
> I don't think providing SIGABRT interception capabilities is woth the
> complexity.
I've since received the opposite feedback from an ISV: they absolutely
want their in-process crash handler to run when glibc aborts in ld.so
due to an assertion failure (or just OOM in the current TLS
implementation). I dislike in-process crash handlers, but it seems this
is still a valid request. So I'm withdrawing my suggestion.
Thanks,
Florian
On 10/04/26 09:38, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> @@ -188,11 +232,11 @@ __assert_perror_fail (int errnum,
>> const char *function)
>> {
>> char errbuf[400];
>> - _dl_fatal_printf ("\
>> + _dl_error_printf ("\
>> Inconsistency detected by ld.so: %s: %u: %s%sUnexpected error: %s.\n",
>> file, line, function ?: "", function ? ": " : "",
>> __strerror_r (errnum, errbuf, sizeof errbuf));
>> -
>> + rtld_abort ();
>> }
>
> Would it make sense to change _dl_fatal_printf to abort instead?
>
> It requires changing the _dl_fatal_printf call in _dl_signal_error
> because we don't want to abort if loading fails due to a missing
> dependency.
But assert usage in dl-load.c (and related functions) are for unexpected
and unhandled situations (like invalid r_debug->state). I do not think
making assert an error on dlopen the expected behavior, I would rather
check every assert usage and see if we can make it a _dl_signal_error.
* Adhemerval Zanella Netto:
> On 10/04/26 09:38, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> @@ -188,11 +232,11 @@ __assert_perror_fail (int errnum,
>>> const char *function)
>>> {
>>> char errbuf[400];
>>> - _dl_fatal_printf ("\
>>> + _dl_error_printf ("\
>>> Inconsistency detected by ld.so: %s: %u: %s%sUnexpected error: %s.\n",
>>> file, line, function ?: "", function ? ": " : "",
>>> __strerror_r (errnum, errbuf, sizeof errbuf));
>>> -
>>> + rtld_abort ();
>>> }
>>
>> Would it make sense to change _dl_fatal_printf to abort instead?
>>
>> It requires changing the _dl_fatal_printf call in _dl_signal_error
>> because we don't want to abort if loading fails due to a missing
>> dependency.
>
> But assert usage in dl-load.c (and related functions) are for unexpected
> and unhandled situations (like invalid r_debug->state). I do not think
> making assert an error on dlopen the expected behavior, I would rather
> check every assert usage and see if we can make it a _dl_signal_error.
Sorry, I meant the path that is executed if there's no exception
handler. That's used for internal dlopen failures of dependencies
during startup (excluding preloading, loading auditors; those failures
are ignored/warning only), and for lazy symbol binding failures later
on.
If a handler is installed, _dl_signal_error does not call
_dl_fatal_printf, and that should not change.
Thanks,
Florian
On 10/04/26 12:49, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
>
>> On 10/04/26 09:38, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> @@ -188,11 +232,11 @@ __assert_perror_fail (int errnum,
>>>> const char *function)
>>>> {
>>>> char errbuf[400];
>>>> - _dl_fatal_printf ("\
>>>> + _dl_error_printf ("\
>>>> Inconsistency detected by ld.so: %s: %u: %s%sUnexpected error: %s.\n",
>>>> file, line, function ?: "", function ? ": " : "",
>>>> __strerror_r (errnum, errbuf, sizeof errbuf));
>>>> -
>>>> + rtld_abort ();
>>>> }
>>>
>>> Would it make sense to change _dl_fatal_printf to abort instead?
>>>
>>> It requires changing the _dl_fatal_printf call in _dl_signal_error
>>> because we don't want to abort if loading fails due to a missing
>>> dependency.
>>
>> But assert usage in dl-load.c (and related functions) are for unexpected
>> and unhandled situations (like invalid r_debug->state). I do not think
>> making assert an error on dlopen the expected behavior, I would rather
>> check every assert usage and see if we can make it a _dl_signal_error.
>
> Sorry, I meant the path that is executed if there's no exception
> handler. That's used for internal dlopen failures of dependencies
> during startup (excluding preloading, loading auditors; those failures
> are ignored/warning only), and for lazy symbol binding failures later
> on.
>
> If a handler is installed, _dl_signal_error does not call
> _dl_fatal_printf, and that should not change.
But the whole point of _dl_fatal_printf is printing the error message
and terminate the process. What this change does it making both the
assert failure during process startup *and* during dlopen to act in
the same manner: printing the error message on STDERR_FILENO and stop
execution as abort was called.
Calling _dl_fatal_printf or not does not really matter, since assert
calls use the barebone mode (no GLRO(dl_debug_fd) and not pid prepending).
@@ -27,23 +27,35 @@
#include <unistd.h>
#include <errno.h>
-/* The rtld startup code calls __rtld_malloc_init_stubs after the
+#include <abort-instr.h>
+#ifndef ABORT_INSTRUCTION
+/* No such instruction is available. */
+# define ABORT_INSTRUCTION
+#endif
+
+/* The rtld startup code calls __rtld_init_stubs after the
first self-relocation to adjust the pointers to the minimal
implementation below. Before the final relocation,
- __rtld_malloc_init_real is called to replace the pointers with the
+ __rtld_init_real is called to replace the pointers with the
real implementation. */
__typeof (calloc) *__rtld_calloc attribute_relro;
__typeof (free) *__rtld_free attribute_relro;
__typeof (malloc) *__rtld_malloc attribute_relro;
__typeof (realloc) *__rtld_realloc attribute_relro;
+__typeof (__assert_fail) *__rtld_assert_fail attribute_relro;
+__typeof (__assert_perror_fail) *__rtld_assert_perror_fail attribute_relro;
+__typeof (__libc_fatal) *__rtld_libc_fatal attribute_relro;
void
-__rtld_malloc_init_stubs (void)
+__rtld_init_stubs (void)
{
__rtld_calloc = &__minimal_calloc;
__rtld_free = &__minimal_free;
__rtld_malloc = &__minimal_malloc;
__rtld_realloc = &__minimal_realloc;
+ __rtld_assert_fail = &__assert_fail;
+ __rtld_assert_perror_fail = &__assert_perror_fail;
+ __rtld_libc_fatal = &__libc_fatal;
}
bool
@@ -56,8 +68,8 @@ __rtld_malloc_is_complete (void)
/* Lookup NAME at VERSION in the scope of MATCH. */
static void *
-lookup_malloc_symbol (struct link_map *main_map, const char *name,
- struct r_found_version *version)
+lookup_symbol (struct link_map *main_map, const char *name,
+ struct r_found_version *version)
{
const ElfW(Sym) *ref = NULL;
@@ -71,8 +83,19 @@ lookup_malloc_symbol (struct link_map *main_map, const char *name,
return _dl_sym_post (result, ref, value, 0, main_map);
}
+static struct r_found_version
+create_r_round_version (const char *name)
+{
+ return (struct r_found_version) {
+ .name = name,
+ .hidden = 0,
+ .hash = _dl_elf_hash (name),
+ .filename = NULL
+ };
+}
+
void
-__rtld_malloc_init_real (struct link_map *main_map)
+__rtld_init_real (struct link_map *main_map)
{
/* We cannot use relocations and initializers for this because the
changes made by __rtld_malloc_init_stubs break REL-style
@@ -82,16 +105,22 @@ __rtld_malloc_init_real (struct link_map *main_map)
rtld relocation (which enables RELRO, after which the pointer
variables cannot be written to). */
- struct r_found_version version;
- version.name = symbol_version_string (libc, GLIBC_2_0);
- version.hidden = 0;
- version.hash = _dl_elf_hash (version.name);
- version.filename = NULL;
+ struct r_found_version version =
+ create_r_round_version (symbol_version_string (libc, GLIBC_2_0));
- void *new_calloc = lookup_malloc_symbol (main_map, "calloc", &version);
- void *new_free = lookup_malloc_symbol (main_map, "free", &version);
- void *new_malloc = lookup_malloc_symbol (main_map, "malloc", &version);
- void *new_realloc = lookup_malloc_symbol (main_map, "realloc", &version);
+ void *new_calloc = lookup_symbol (main_map, "calloc", &version);
+ void *new_free = lookup_symbol (main_map, "free", &version);
+ void *new_malloc = lookup_symbol (main_map, "malloc", &version);
+ void *new_realloc = lookup_symbol (main_map, "realloc", &version);
+ void *new_assert_fail = lookup_symbol (main_map, "__assert_fail", &version);
+ void *new_assert_perror_fail = lookup_symbol (main_map, "__assert_perror_fail",
+ &version);
+
+ struct r_found_version version_private =
+ create_r_round_version (symbol_version_string (libc, GLIBC_PRIVATE));
+
+ void *new_libc_fatal = lookup_symbol (main_map, "__libc_fatal",
+ &version_private);
/* Update the pointers in one go, so that any internal allocations
performed by lookup_malloc_symbol see a consistent
@@ -100,6 +129,9 @@ __rtld_malloc_init_real (struct link_map *main_map)
__rtld_free = new_free;
__rtld_malloc = new_malloc;
__rtld_realloc = new_realloc;
+ __rtld_assert_fail = new_assert_fail;
+ __rtld_assert_perror_fail = new_assert_perror_fail;
+ __rtld_libc_fatal = new_libc_fatal;
}
@@ -114,6 +146,18 @@ __sigjmp_save (sigjmp_buf env, int savemask __attribute__ ((unused)))
return 0;
}
+_Noreturn static void
+rtld_abort (void)
+{
+ __raise_direct (SIGABRT);
+ /* Assume there is no SIGABRT handler installed during process startup, so
+ there is no need to add a fallback to change the signal disposition, as
+ the libc implementation does. */
+ ABORT_INSTRUCTION;
+
+ _exit (127);
+}
+
/* Define our own version of the internal function used by strerror. We
only provide the messages for some common errors. This avoids pulling
in the whole error list. */
@@ -172,11 +216,11 @@ void weak_function
__assert_fail (const char *assertion,
const char *file, unsigned int line, const char *function)
{
- _dl_fatal_printf ("\
+ _dl_error_printf ("\
Inconsistency detected by ld.so: %s: %u: %s%sAssertion `%s' failed!\n",
file, line, function ?: "", function ? ": " : "",
assertion);
-
+ rtld_abort ();
}
# ifndef NO_RTLD_HIDDEN
rtld_hidden_weak (__assert_fail)
@@ -188,11 +232,11 @@ __assert_perror_fail (int errnum,
const char *function)
{
char errbuf[400];
- _dl_fatal_printf ("\
+ _dl_error_printf ("\
Inconsistency detected by ld.so: %s: %u: %s%sUnexpected error: %s.\n",
file, line, function ?: "", function ? ": " : "",
__strerror_r (errnum, errbuf, sizeof errbuf));
-
+ rtld_abort ();
}
# ifndef NO_RTLD_HIDDEN
rtld_hidden_weak (__assert_perror_fail)
@@ -451,7 +451,7 @@ _dl_start_final (void *arg, struct dl_start_final_info *info)
{
ElfW(Addr) start_addr;
- __rtld_malloc_init_stubs ();
+ __rtld_init_stubs ();
/* Do not use an initializer for these members because it would
interfere with __rtld_static_init. */
@@ -2342,7 +2342,7 @@ dl_main (const ElfW(Phdr) *phdr,
rtld_timer_accum (&relocate_time, start);
__rtld_mutex_init ();
- __rtld_malloc_init_real (main_map);
+ __rtld_init_real (main_map);
/* Update copy-relocated _r_debug if necessary. */
_dl_debug_post_relocate (main_map);
@@ -1,6 +1,8 @@
#include <assert/assert.h>
#ifndef _ISOMAC
+# include <rtld-assert.h>
+
/* This prints an "Assertion failed" message and aborts.
In installed assert.h this is only conditionally declared,
so it has to be repeated here. */
new file mode 100644
@@ -0,0 +1,55 @@
+/* Redirection of assert inside the dynamic linker.
+ Copyright (C) 2026 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _RTLD_ASSERT_H
+#define _RTLD_ASSERT_H
+
+#if IS_IN (rtld)
+
+extern void (*__rtld_assert_fail)(const char *, const char *, unsigned int,
+ const char *) attribute_hidden;
+extern void (*__rtld_assert_perror_fail)(int, const char *, unsigned int,
+ const char *) attribute_hidden;
+extern void (*__rtld_libc_fatal)(const char *) attribute_hidden;
+
+__extern_inline _Noreturn void
+__assert_fail (const char *assertion, const char *file, unsigned int line,
+ const char *function)
+{
+ __rtld_assert_fail (assertion, file, line, function);
+ __builtin_unreachable ();
+}
+
+__extern_inline _Noreturn void
+__assert_perror_fail (int errnum, const char *file, unsigned int line,
+ const char *function)
+{
+ __rtld_assert_perror_fail (errnum, file, line, function);
+ __builtin_unreachable ();
+}
+
+__extern_inline _Noreturn void
+__libc_fatal (const char *message)
+{
+ __rtld_libc_fatal (message);
+ __builtin_unreachable ();
+}
+
+#endif
+
+#endif
@@ -62,20 +62,6 @@ realloc (void *ptr, size_t size)
return __rtld_realloc (ptr, size);
}
-/* Called after the first self-relocation to activate the minimal malloc
- implementation. */
-void __rtld_malloc_init_stubs (void) attribute_hidden;
-
-/* Return false if the active malloc is the ld.so minimal malloc, true
- if it is the full implementation from libc.so. */
-_Bool __rtld_malloc_is_complete (void) attribute_hidden;
-
-/* Called shortly before the final self-relocation (when RELRO
- variables are still writable) to activate the real malloc
- implementation. MAIN_MAP is the link map of the executable. */
-struct link_map;
-void __rtld_malloc_init_real (struct link_map *main_map) attribute_hidden;
-
#else /* !IS_IN (rtld) */
/* This allows static/non-rtld builds to get a pointer to the
@@ -169,6 +169,8 @@ extern int __gen_tempname (char *__tmpl, int __suffixlen, int __flags,
# define __GT_DIR 1 /* create a directory */
# define __GT_NOCREATE 2 /* just find a name not currently in use */
+/* Enable rtld __libc_fatal minimal implementation. */
+# include <rtld-assert.h>
/* Print out MESSAGE (which should end with a newline) on the error output
and abort. */
extern void __libc_fatal (const char *__message)
@@ -1436,6 +1436,22 @@ _dl_audit_objclose (struct link_map *l)
}
#endif /* !SHARED */
+#if IS_IN (rtld)
+/* Called after the first self-relocation to activate the minimal malloc
+ implementation. */
+void __rtld_init_stubs (void) attribute_hidden;
+
+/* Return false if the active malloc is the ld.so minimal malloc, true
+ if it is the full implementation from libc.so. */
+_Bool __rtld_malloc_is_complete (void) attribute_hidden;
+
+/* Called shortly before the final self-relocation (when RELRO
+ variables are still writable) to activate the real malloc
+ implementation. MAIN_MAP is the link map of the executable. */
+struct link_map;
+void __rtld_init_real (struct link_map *main_map) attribute_hidden;
+#endif
+
#if __PTHREAD_NPTL && defined SHARED
/* Recursive locking implementation for use within the dynamic loader.
Used to define the __rtld_lock_lock_recursive and
@@ -334,3 +334,9 @@ endif
ifeq ($(subdir),io)
tests-unsupported += test-lfs
endif
+
+ifeq ($(subdir),signal)
+sysdep_routines += \
+ raise_direct \
+ # sysdep_routines
+endif
new file mode 100644
@@ -0,0 +1,29 @@
+/* Internal function to send a signal to itself. Hurd version.
+ Copyright (C) 2026 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <hurd.h>
+#include <signal.h>
+#include <unistd.h>
+
+int
+__raise_direct (int sig)
+{
+ /* TODO: proper implement it to work correctly on dynamic linker
+ startup. */
+ return 0;
+}