From c6d20445330c3059f3f4d5984d8fd61d179cc921 Mon Sep 17 00:00:00 2001 From: Ramon Nou Date: Fri, 18 Jul 2025 18:03:17 +0200 Subject: [PATCH 1/2] fix syscall tls recursion --- src/client/intercept.cpp | 73 ++++++++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/src/client/intercept.cpp b/src/client/intercept.cpp index 1ae6e9860..32c9c92b0 100644 --- a/src/client/intercept.cpp +++ b/src/client/intercept.cpp @@ -36,7 +36,7 @@ SPDX-License-Identifier: LGPL-3.0-or-later */ - +#include #include #include #include @@ -74,7 +74,13 @@ void (*intercept_hook_point_post_kernel)(long syscall_number, long arg0, #endif namespace { -thread_local bool reentrance_guard_flag; +static pthread_key_t reentrance_guard_key; +static pthread_once_t key_once_control = PTHREAD_ONCE_INIT; +static void make_key() { + // This function will be called exactly once per process. + pthread_key_create(&reentrance_guard_key, NULL); +} + thread_local gkfs::syscall::info saved_syscall_info; constexpr void @@ -533,8 +539,9 @@ hook(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, case SYS_execve: // If we do not set this to false, we are in trouble with vforks - reentrance_guard_flag = false; - *result = syscall_no_intercept_wrapper( +pthread_once(&key_once_control, make_key); + pthread_setspecific(reentrance_guard_key, NULL); + *result = syscall_no_intercept_wrapper( syscall_number, reinterpret_cast(arg0), reinterpret_cast(arg1), reinterpret_cast(arg2)); @@ -543,8 +550,9 @@ hook(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, #ifdef SYS_execveat case SYS_execveat: // If we do not set this to false, we are in trouble with vforks - reentrance_guard_flag = false; - *result = syscall_no_intercept_wrapper( +pthread_once(&key_once_control, make_key); + pthread_setspecific(reentrance_guard_key, NULL); + *result = syscall_no_intercept_wrapper( syscall_number, arg0, reinterpret_cast(arg1), reinterpret_cast(arg2), reinterpret_cast(arg3), arg4); @@ -1011,12 +1019,19 @@ socketcall_wrapper(long syscall_number, long& arg0, long& arg1, long& arg2, } #endif - void hook_forwarded_syscall(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, long arg5, long result) { + + pthread_once(&key_once_control, make_key); + if (pthread_getspecific(reentrance_guard_key) != NULL) { + return; + } + pthread_setspecific(reentrance_guard_key, (void*) 1); + if(::get_current_syscall_info() == gkfs::syscall::no_info) { + pthread_setspecific(reentrance_guard_key, NULL); return; } @@ -1029,6 +1044,8 @@ hook_forwarded_syscall(long syscall_number, long arg0, long arg1, long arg2, syscall_number, args, result); ::reset_current_syscall_info(); + + pthread_setspecific(reentrance_guard_key, NULL); } void @@ -1042,12 +1059,13 @@ hook_clone_at_child(unsigned long flags, void* child_stack, int* ptid, static_cast(newtls), 0}; #endif - reentrance_guard_flag = true; + pthread_once(&key_once_control, make_key); + pthread_setspecific(reentrance_guard_key, (void*) 1); LOG(SYSCALL, ::get_current_syscall_info() | gkfs::syscall::executed, SYS_clone, args, 0); - reentrance_guard_flag = false; + pthread_setspecific(reentrance_guard_key, NULL); } void @@ -1061,18 +1079,20 @@ hook_clone_at_parent(unsigned long flags, void* child_stack, int* ptid, static_cast(newtls), 0}; #endif - reentrance_guard_flag = true; + pthread_once(&key_once_control, make_key); + pthread_setspecific(reentrance_guard_key, (void*) 1); LOG(SYSCALL, ::get_current_syscall_info() | gkfs::syscall::executed, SYS_clone, args, returned_pid); - reentrance_guard_flag = false; + pthread_setspecific(reentrance_guard_key, NULL); } } // namespace namespace gkfs::preload { +// This function is inside 'namespace gkfs::preload' int internal_hook_guard_wrapper(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, long arg5, @@ -1085,7 +1105,8 @@ internal_hook_guard_wrapper(long syscall_number, long arg0, long arg1, arg3, arg4, arg5); #endif - if(reentrance_guard_flag) { + pthread_once(&key_once_control, make_key); + if (pthread_getspecific(reentrance_guard_key) != NULL) { ::save_current_syscall_info(gkfs::syscall::from_internal_code | gkfs::syscall::to_kernel | gkfs::syscall::not_executed); @@ -1094,15 +1115,14 @@ internal_hook_guard_wrapper(long syscall_number, long arg0, long arg1, int was_hooked = 0; - reentrance_guard_flag = true; + pthread_setspecific(reentrance_guard_key, (void*) 1); was_hooked = hook_internal(syscall_number, arg0, arg1, arg2, arg3, arg4, arg5, syscall_return_value); - reentrance_guard_flag = false; + pthread_setspecific(reentrance_guard_key, NULL); return was_hooked; } - /* * hook_guard_wrapper -- a wrapper which can notice reentrance. * @@ -1122,6 +1142,17 @@ hook_guard_wrapper(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, long arg5, long* syscall_return_value) { + // --- START OF REVISED FIX for hook_guard_wrapper --- + pthread_once(&key_once_control, make_key); // Ensure the key is created + if (pthread_getspecific(reentrance_guard_key) != NULL) { + // If the guard is set, forward the syscall to the kernel and return. + return gkfs::syscall::forward_to_kernel; + } + // Set the guard to a non-NULL value. + pthread_setspecific(reentrance_guard_key, (void*) 1); + // --- END OF REVISED FIX for hook_guard_wrapper --- + + assert(CTX->interception_enabled()); #ifdef SYS_socketcall @@ -1132,19 +1163,11 @@ hook_guard_wrapper(long syscall_number, long arg0, long arg1, long arg2, int was_hooked = 0; - if(reentrance_guard_flag) { - - was_hooked = hook_internal(syscall_number, arg0, arg1, arg2, arg3, arg4, - arg5, syscall_return_value); - return was_hooked; - } - - reentrance_guard_flag = true; - was_hooked = ::hook(syscall_number, arg0, arg1, arg2, arg3, arg4, arg5, syscall_return_value); - reentrance_guard_flag = false; + // Unset the guard before returning + pthread_setspecific(reentrance_guard_key, NULL); return was_hooked; } -- GitLab From 5b25f4fdb2f0527e585e4c251ac8932aa0ad8143 Mon Sep 17 00:00:00 2001 From: Ramon Nou Date: Wed, 6 Aug 2025 10:35:53 +0200 Subject: [PATCH 2/2] fix formatting --- CHANGELOG.md | 2 ++ src/client/intercept.cpp | 24 +++++++++++------------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbf41f131..13ce7c8b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - execve set reentrant_guard as true, so once it returns (vfork) the parent had syscall disabled. - debug should be still set to debug level on syscall for vfork. - In MN5 gkfs_libc.hpp needs to have the libc signatures. + - Thread local storage produces some seg fault in Mogon-2 with syscall ([!266](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/266)) + - using pthread functions ## [0.9.4] - 2025-03 diff --git a/src/client/intercept.cpp b/src/client/intercept.cpp index 32c9c92b0..35df1cf9a 100644 --- a/src/client/intercept.cpp +++ b/src/client/intercept.cpp @@ -76,7 +76,8 @@ namespace { static pthread_key_t reentrance_guard_key; static pthread_once_t key_once_control = PTHREAD_ONCE_INIT; -static void make_key() { +static void +make_key() { // This function will be called exactly once per process. pthread_key_create(&reentrance_guard_key, NULL); } @@ -539,9 +540,9 @@ hook(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, case SYS_execve: // If we do not set this to false, we are in trouble with vforks -pthread_once(&key_once_control, make_key); + pthread_once(&key_once_control, make_key); pthread_setspecific(reentrance_guard_key, NULL); - *result = syscall_no_intercept_wrapper( + *result = syscall_no_intercept_wrapper( syscall_number, reinterpret_cast(arg0), reinterpret_cast(arg1), reinterpret_cast(arg2)); @@ -550,9 +551,9 @@ pthread_once(&key_once_control, make_key); #ifdef SYS_execveat case SYS_execveat: // If we do not set this to false, we are in trouble with vforks -pthread_once(&key_once_control, make_key); + pthread_once(&key_once_control, make_key); pthread_setspecific(reentrance_guard_key, NULL); - *result = syscall_no_intercept_wrapper( + *result = syscall_no_intercept_wrapper( syscall_number, arg0, reinterpret_cast(arg1), reinterpret_cast(arg2), reinterpret_cast(arg3), arg4); @@ -1022,9 +1023,9 @@ socketcall_wrapper(long syscall_number, long& arg0, long& arg1, long& arg2, void hook_forwarded_syscall(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, long arg5, long result) { - + pthread_once(&key_once_control, make_key); - if (pthread_getspecific(reentrance_guard_key) != NULL) { + if(pthread_getspecific(reentrance_guard_key) != NULL) { return; } pthread_setspecific(reentrance_guard_key, (void*) 1); @@ -1044,7 +1045,7 @@ hook_forwarded_syscall(long syscall_number, long arg0, long arg1, long arg2, syscall_number, args, result); ::reset_current_syscall_info(); - + pthread_setspecific(reentrance_guard_key, NULL); } @@ -1106,7 +1107,7 @@ internal_hook_guard_wrapper(long syscall_number, long arg0, long arg1, #endif pthread_once(&key_once_control, make_key); - if (pthread_getspecific(reentrance_guard_key) != NULL) { + if(pthread_getspecific(reentrance_guard_key) != NULL) { ::save_current_syscall_info(gkfs::syscall::from_internal_code | gkfs::syscall::to_kernel | gkfs::syscall::not_executed); @@ -1142,16 +1143,13 @@ hook_guard_wrapper(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, long arg5, long* syscall_return_value) { - // --- START OF REVISED FIX for hook_guard_wrapper --- pthread_once(&key_once_control, make_key); // Ensure the key is created - if (pthread_getspecific(reentrance_guard_key) != NULL) { + if(pthread_getspecific(reentrance_guard_key) != NULL) { // If the guard is set, forward the syscall to the kernel and return. return gkfs::syscall::forward_to_kernel; } // Set the guard to a non-NULL value. pthread_setspecific(reentrance_guard_key, (void*) 1); - // --- END OF REVISED FIX for hook_guard_wrapper --- - assert(CTX->interception_enabled()); -- GitLab