From 6d564f0a194ffc655061fe1bd35333fbfcafb488 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 15 Jan 2024 13:36:29 -0500 Subject: [PATCH] Fix: libc wrapper: use initial-exec for malloc_nesting TLS Use the initial-exec TLS model for the malloc_nesting nesting guard variable to ensure that the glibc implementation of the TLS access don't trigger infinite recursion by calling the memory allocator wrapper functions, which can happen with global-dynamic. Considering that the libc wrapper is meant to be loaded with LD_PRELOAD anyway (never with dlopen(3)), we always expect the libc to have enough space to hold the malloc_nesting variable. In addition to change the malloc_nesting from global-dynamic to initial-exec, this removes the URCU TLS compatibility layer from the libc wrapper, which is a good thing: this compatibility layer relies on pthread key and calloc internally, which makes it a bad fit for TLS accesses guarding access to malloc wrappers, due to possible infinite recursion. Link: https://lists.lttng.org/pipermail/lttng-dev/2024-January/030697.html Reported-by: Florian Weimer Signed-off-by: Mathieu Desnoyers Change-Id: I72c42bc09c1a06e2922b184b85abeb9c94200ee2 --- liblttng-ust-libc-wrapper/lttng-ust-malloc.c | 58 +++++++++----------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/liblttng-ust-libc-wrapper/lttng-ust-malloc.c b/liblttng-ust-libc-wrapper/lttng-ust-malloc.c index 9e7d1e63..84641dda 100644 --- a/liblttng-ust-libc-wrapper/lttng-ust-malloc.c +++ b/liblttng-ust-libc-wrapper/lttng-ust-malloc.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -92,13 +91,13 @@ void ust_malloc_spin_unlock(pthread_mutex_t *lock) uatomic_set(&ust_malloc_lock, 0); } -#define calloc static_calloc -#define pthread_mutex_lock ust_malloc_spin_lock -#define pthread_mutex_unlock ust_malloc_spin_unlock -static DEFINE_URCU_TLS(int, malloc_nesting); -#undef pthread_mutex_unlock -#undef pthread_mutex_lock -#undef calloc +/* + * Use initial-exec TLS model for the malloc_nesting nesting guard + * variable to ensure that the glibc implementation of the TLS access + * don't trigger infinite recursion by calling the memory allocator + * wrapper functions, which could happen with global-dynamic. + */ +static __thread __attribute__((tls_model("initial-exec"))) int malloc_nesting; /* * Static allocator to use when initially executing dlsym(). It keeps a @@ -257,7 +256,7 @@ void *malloc(size_t size) { void *retval; - URCU_TLS(malloc_nesting)++; + malloc_nesting++; if (cur_alloc.malloc == NULL) { lookup_all_symbols(); if (cur_alloc.malloc == NULL) { @@ -266,17 +265,17 @@ void *malloc(size_t size) } } retval = cur_alloc.malloc(size); - if (URCU_TLS(malloc_nesting) == 1) { + if (malloc_nesting == 1) { tracepoint(lttng_ust_libc, malloc, size, retval, LTTNG_UST_CALLER_IP()); } - URCU_TLS(malloc_nesting)--; + malloc_nesting--; return retval; } void free(void *ptr) { - URCU_TLS(malloc_nesting)++; + malloc_nesting++; /* * Check whether the memory was allocated with * static_calloc_align, in which case there is nothing to free. @@ -286,7 +285,7 @@ void free(void *ptr) goto end; } - if (URCU_TLS(malloc_nesting) == 1) { + if (malloc_nesting == 1) { tracepoint(lttng_ust_libc, free, ptr, LTTNG_UST_CALLER_IP()); } @@ -300,14 +299,14 @@ void free(void *ptr) } cur_alloc.free(ptr); end: - URCU_TLS(malloc_nesting)--; + malloc_nesting--; } void *calloc(size_t nmemb, size_t size) { void *retval; - URCU_TLS(malloc_nesting)++; + malloc_nesting++; if (cur_alloc.calloc == NULL) { lookup_all_symbols(); if (cur_alloc.calloc == NULL) { @@ -316,11 +315,11 @@ void *calloc(size_t nmemb, size_t size) } } retval = cur_alloc.calloc(nmemb, size); - if (URCU_TLS(malloc_nesting) == 1) { + if (malloc_nesting == 1) { tracepoint(lttng_ust_libc, calloc, nmemb, size, retval, LTTNG_UST_CALLER_IP()); } - URCU_TLS(malloc_nesting)--; + malloc_nesting--; return retval; } @@ -328,7 +327,7 @@ void *realloc(void *ptr, size_t size) { void *retval; - URCU_TLS(malloc_nesting)++; + malloc_nesting++; /* * Check whether the memory was allocated with * static_calloc_align, in which case there is nothing @@ -369,11 +368,11 @@ void *realloc(void *ptr, size_t size) } retval = cur_alloc.realloc(ptr, size); end: - if (URCU_TLS(malloc_nesting) == 1) { + if (malloc_nesting == 1) { tracepoint(lttng_ust_libc, realloc, ptr, size, retval, LTTNG_UST_CALLER_IP()); } - URCU_TLS(malloc_nesting)--; + malloc_nesting--; return retval; } @@ -381,7 +380,7 @@ void *memalign(size_t alignment, size_t size) { void *retval; - URCU_TLS(malloc_nesting)++; + malloc_nesting++; if (cur_alloc.memalign == NULL) { lookup_all_symbols(); if (cur_alloc.memalign == NULL) { @@ -390,12 +389,12 @@ void *memalign(size_t alignment, size_t size) } } retval = cur_alloc.memalign(alignment, size); - if (URCU_TLS(malloc_nesting) == 1) { + if (malloc_nesting == 1) { tracepoint(lttng_ust_libc, memalign, alignment, size, retval, LTTNG_UST_CALLER_IP()); } - URCU_TLS(malloc_nesting)--; + malloc_nesting--; return retval; } @@ -403,7 +402,7 @@ int posix_memalign(void **memptr, size_t alignment, size_t size) { int retval; - URCU_TLS(malloc_nesting)++; + malloc_nesting++; if (cur_alloc.posix_memalign == NULL) { lookup_all_symbols(); if (cur_alloc.posix_memalign == NULL) { @@ -412,21 +411,15 @@ int posix_memalign(void **memptr, size_t alignment, size_t size) } } retval = cur_alloc.posix_memalign(memptr, alignment, size); - if (URCU_TLS(malloc_nesting) == 1) { + if (malloc_nesting == 1) { tracepoint(lttng_ust_libc, posix_memalign, *memptr, alignment, size, retval, LTTNG_UST_CALLER_IP()); } - URCU_TLS(malloc_nesting)--; + malloc_nesting--; return retval; } -static -void lttng_ust_fixup_malloc_nesting_tls(void) -{ - asm volatile ("" : : "m" (URCU_TLS(malloc_nesting))); -} - __attribute__((constructor)) void lttng_ust_malloc_wrapper_init(void) { @@ -434,7 +427,6 @@ void lttng_ust_malloc_wrapper_init(void) if (cur_alloc.calloc) { return; } - lttng_ust_fixup_malloc_nesting_tls(); /* * Ensure the allocator is in place before the process becomes * multithreaded. -- 2.34.1