| Message ID | 20240611153220.165430-5-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for memory sealing | expand |
* Adhemerval Zanella: > The libgcc_s.so can also be sealed. The library is loaded once > and not unloaded during process execution (only for memory debug > with __libc_unwind_link_freeres). The unwind-link change to use RTLD_NODELETE could go in separately. > diff --git a/misc/unwind-link.c b/misc/unwind-link.c > index 213a0162a4..7267ecbec3 100644 > --- a/misc/unwind-link.c > +++ b/misc/unwind-link.c > @@ -100,7 +100,8 @@ __libc_unwind_link_get (void) > > __libc_lock_lock (lock); > if (atomic_load_relaxed (&global_libgcc_handle) != NULL) > - /* This thread lost the race. Clean up. */ > + /* This thread lost the race. Drop the l_direct_opencount and issue > + the debug log. */ > __libc_dlclose (local_libgcc_handle); > else > { I don't understand what “debug log” means in this context. Thanks, Florian
On 12/06/24 06:54, Florian Weimer wrote: > * Adhemerval Zanella: > >> The libgcc_s.so can also be sealed. The library is loaded once >> and not unloaded during process execution (only for memory debug >> with __libc_unwind_link_freeres). > > The unwind-link change to use RTLD_NODELETE could go in separately. Ok, I will send a separate patch. > >> diff --git a/misc/unwind-link.c b/misc/unwind-link.c >> index 213a0162a4..7267ecbec3 100644 >> --- a/misc/unwind-link.c >> +++ b/misc/unwind-link.c > >> @@ -100,7 +100,8 @@ __libc_unwind_link_get (void) >> >> __libc_lock_lock (lock); >> if (atomic_load_relaxed (&global_libgcc_handle) != NULL) >> - /* This thread lost the race. Clean up. */ >> + /* This thread lost the race. Drop the l_direct_opencount and issue >> + the debug log. */ >> __libc_dlclose (local_libgcc_handle); >> else >> { > > I don't understand what “debug log” means in this context. Sorry, I meant the __libc_unwind_link_freeres usually triggered by memory profilers. With sealing the dlclose won't unmap the memory.
* Adhemerval Zanella Netto: > On 12/06/24 06:54, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> The libgcc_s.so can also be sealed. The library is loaded once >>> and not unloaded during process execution (only for memory debug >>> with __libc_unwind_link_freeres). >> >> The unwind-link change to use RTLD_NODELETE could go in separately. > > Ok, I will send a separate patch. > >> >>> diff --git a/misc/unwind-link.c b/misc/unwind-link.c >>> index 213a0162a4..7267ecbec3 100644 >>> --- a/misc/unwind-link.c >>> +++ b/misc/unwind-link.c >> >>> @@ -100,7 +100,8 @@ __libc_unwind_link_get (void) >>> >>> __libc_lock_lock (lock); >>> if (atomic_load_relaxed (&global_libgcc_handle) != NULL) >>> - /* This thread lost the race. Clean up. */ >>> + /* This thread lost the race. Drop the l_direct_opencount and issue >>> + the debug log. */ >>> __libc_dlclose (local_libgcc_handle); >>> else >>> { >> >> I don't understand what “debug log” means in this context. > > Sorry, I meant the __libc_unwind_link_freeres usually triggered by > memory profilers. With sealing the dlclose won't unmap the memory. I still don't understand the comment. We can still deallocate the helper data structures. With the switch to read-only link maps, most of the allocations will be hidden from malloc tracing anyway and no longer appear as leaks. Thanks, Florian
On 12/06/24 14:50, Florian Weimer wrote: > * Adhemerval Zanella Netto: > >> On 12/06/24 06:54, Florian Weimer wrote: >>> * Adhemerval Zanella: >>> >>>> The libgcc_s.so can also be sealed. The library is loaded once >>>> and not unloaded during process execution (only for memory debug >>>> with __libc_unwind_link_freeres). >>> >>> The unwind-link change to use RTLD_NODELETE could go in separately. >> >> Ok, I will send a separate patch. >> >>> >>>> diff --git a/misc/unwind-link.c b/misc/unwind-link.c >>>> index 213a0162a4..7267ecbec3 100644 >>>> --- a/misc/unwind-link.c >>>> +++ b/misc/unwind-link.c >>> >>>> @@ -100,7 +100,8 @@ __libc_unwind_link_get (void) >>>> >>>> __libc_lock_lock (lock); >>>> if (atomic_load_relaxed (&global_libgcc_handle) != NULL) >>>> - /* This thread lost the race. Clean up. */ >>>> + /* This thread lost the race. Drop the l_direct_opencount and issue >>>> + the debug log. */ >>>> __libc_dlclose (local_libgcc_handle); >>>> else >>>> { >>> >>> I don't understand what “debug log” means in this context. >> >> Sorry, I meant the __libc_unwind_link_freeres usually triggered by >> memory profilers. With sealing the dlclose won't unmap the memory. > > I still don't understand the comment. > > We can still deallocate the helper data structures. With the switch to > read-only link maps, most of the allocations will be hidden from malloc > tracing anyway and no longer appear as leaks. I am not sure if valgrind or any memory profilers won't complain about lingering map segments, but you are right that at least regarding _dl_unmap_segments we don't really check the unmmap result.
diff --git a/include/dlfcn.h b/include/dlfcn.h index f49ee1b0c9..06e2ecbdd2 100644 --- a/include/dlfcn.h +++ b/include/dlfcn.h @@ -50,6 +50,8 @@ extern char **__libc_argv attribute_hidden; better error handling semantics for the library. */ #define __libc_dlopen(name) \ __libc_dlopen_mode (name, RTLD_NOW | __RTLD_DLOPEN) +#define __libc_dlopen_nodelete(name) \ + __libc_dlopen_mode (name, RTLD_NODELETE | RTLD_NOW | __RTLD_DLOPEN) extern void *__libc_dlopen_mode (const char *__name, int __mode) attribute_hidden; extern void *__libc_dlsym (void *__map, const char *__name) diff --git a/manual/tunables.texi b/manual/tunables.texi index 26fba6641d..be36d52cf9 100644 --- a/manual/tunables.texi +++ b/manual/tunables.texi @@ -381,6 +381,10 @@ Any preload libraries. @item Any library loaded with @code{dlopen} with @code{RTLD_NODELETE} flag. + +@item +Any runtime library used for process unwind (such as required by @code{backtrace} +or @code{pthread_exit}). @end itemize The tunable accepts three diferent values: @samp{0} where sealing is disabled, diff --git a/misc/unwind-link.c b/misc/unwind-link.c index 213a0162a4..7267ecbec3 100644 --- a/misc/unwind-link.c +++ b/misc/unwind-link.c @@ -48,7 +48,7 @@ __libc_unwind_link_get (void) /* Initialize a copy of the data, so that we do not need about unlocking in case the dynamic loader somehow triggers unwinding. */ - void *local_libgcc_handle = __libc_dlopen (LIBGCC_S_SO); + void *local_libgcc_handle = __libc_dlopen_nodelete (LIBGCC_S_SO); if (local_libgcc_handle == NULL) { __libc_lock_unlock (lock); @@ -100,7 +100,8 @@ __libc_unwind_link_get (void) __libc_lock_lock (lock); if (atomic_load_relaxed (&global_libgcc_handle) != NULL) - /* This thread lost the race. Clean up. */ + /* This thread lost the race. Drop the l_direct_opencount and issue + the debug log. */ __libc_dlclose (local_libgcc_handle); else { diff --git a/sysdeps/unix/sysv/linux/tst-dl_mseal.c b/sysdeps/unix/sysv/linux/tst-dl_mseal.c index 72a33d04c7..da1a3ebe5a 100644 --- a/sysdeps/unix/sysv/linux/tst-dl_mseal.c +++ b/sysdeps/unix/sysv/linux/tst-dl_mseal.c @@ -19,6 +19,7 @@ #include <array_length.h> #include <errno.h> #include <getopt.h> +#include <gnu/lib-names.h> #include <inttypes.h> #include <libgen.h> #include <stdio.h> @@ -31,6 +32,7 @@ #include <support/support.h> #include <support/xdlfcn.h> #include <support/xstdio.h> +#include <support/xthread.h> #define LIB_PRELOAD "lib-tst-dl_mseal-preload.so" @@ -70,6 +72,7 @@ static const char *expected_sealed_libs[] = LIB_NEEDED_2, LIB_DLOPEN_NODELETE, LIB_DLOPEN_NODELETE_DEP, + LIBGCC_S_SO, #endif "[vdso]", }; @@ -100,6 +103,13 @@ is_in_string_list (const char *s, const char *const list[], size_t len) return -1; } +static void * +tf (void *closure) +{ + pthread_exit (NULL); + return NULL; +} + static int handle_restart (void) { @@ -108,6 +118,9 @@ handle_restart (void) xdlopen (LIB_DLOPEN_DEFAULT, RTLD_NOW); #endif + /* pthread_exit will load LIBGCC_S_SO. */ + xpthread_join (xpthread_create (NULL, tf, NULL)); + FILE *fp = xfopen ("/proc/self/maps", "r"); char *line = NULL; size_t linesiz = 0;