From cc976d128c109733ba31a35e0c20f706d10f3a93 Mon Sep 17 00:00:00 2001 From: Waldemar Kozaczuk Date: Mon, 18 Dec 2023 14:28:40 -0500 Subject: [PATCH] signals: switch to app TLS if available This patch changes logic around executing user signal handler in kill() to make new signal handler thread use app TLS of an application thread if available. Normally, application threads share thread local storage area (TLS) with kernel or use one created by kernel. But when running statically linked executables or dynamically ones with Linux dynamic linker, the application threads create TLS on its own and user signal handler may reference thread local variables which do not exist in the TLS of the new signal handler thread. As a result the app would crash like so: ``` 9 0x000000004030bd54 in page_fault (ef=0x40007fe83088) at arch/x64/mmu.cc:42 10 11 __GI___pthread_cleanup_upto (target=target@entry=0x200000205080 , targetframe=targetframe@entry=0x40007fe93fc8 "\005\"D\177") at ./nptl/pthread_cleanup_upto.c:32 12 0x000020007f44232c in _longjmp_unwind (env=env@entry=0x200000205080 , val=val@entry=1) at ../sysdeps/nptl/jmp-unwind.c:27 13 0x000020007f442205 in __libc_siglongjmp (env=0x200000205080 , val=1) at ../setjmp/longjmp.c:30 14 0x0000200000202543 in sigend_handler (sig=2) at main.c:121 15 0x000000004037f0ee in sched::thread::main (this=0x40007fe7e040) at core/sched.cc:1415 16 sched::thread_main_c (t=0x40007fe7e040) at arch/x64/arch-switch.hh:377 17 0x000000004030bc52 in thread_main () at arch/x64/entry.S:161 ``` This patch applies a bit of trickery (potentially dangerous if user handler tries to write to TLS) and select an application TLS of a current thread or one of the other application threads and makes it available to the new signal handler thread. The signal handler thread in such case would switch from kernel TLS to app TLS before executing handler routine. This patch makes following tests pass when running with Linux dynamic linker: - tst-kill - tst-sigwait - tst-sigaction Refers #1278 Signed-off-by: Waldemar Kozaczuk --- arch/x64/tls-switch.hh | 22 ++++++++++++++++++++++ core/sched.cc | 10 ++++++++++ include/osv/sched.hh | 2 ++ libc/signal.cc | 29 +++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+) diff --git a/arch/x64/tls-switch.hh b/arch/x64/tls-switch.hh index 8be619dbef..651b7d0cde 100644 --- a/arch/x64/tls-switch.hh +++ b/arch/x64/tls-switch.hh @@ -48,6 +48,28 @@ public: } } }; +// +//Simple RAII utility classes that implement the logic to switch +//fsbase to the specified app address and back to the kernel one +class user_tls_switch { + thread_control_block *_kernel_tcb; +public: + user_tls_switch() { + asm volatile ( "movq %%gs:16, %0\n\t" : "=r"(_kernel_tcb)); + + //Switch to app tcb if app tcb present + if (_kernel_tcb->app_tcb) { + set_fsbase(reinterpret_cast(_kernel_tcb->app_tcb)); + } + } + + ~user_tls_switch() { + //Switch to kernel tcb if app tcb present + if (_kernel_tcb->app_tcb) { + set_fsbase(reinterpret_cast(_kernel_tcb->self)); + } + } +}; } diff --git a/core/sched.cc b/core/sched.cc index bdffe0bea9..61c388bf62 100644 --- a/core/sched.cc +++ b/core/sched.cc @@ -2088,6 +2088,16 @@ void with_thread_by_id(unsigned id, std::function f) { } } +thread *find_first_app_thread(std::function f) { + WITH_LOCK(thread_map_mutex) { + for (auto th : thread_map) { + if(th.second->is_app() && f(*th.second)) { + return th.second; + } + } + } + return nullptr; +} } diff --git a/include/osv/sched.hh b/include/osv/sched.hh index d2c3b9352a..b495f43ae8 100644 --- a/include/osv/sched.hh +++ b/include/osv/sched.hh @@ -1552,6 +1552,8 @@ void with_all_threads(std::function); // should return quickly. void with_thread_by_id(unsigned id, std::function); +thread *find_first_app_thread(std::function f); + } #endif /* SCHED_HH_ */ diff --git a/libc/signal.cc b/libc/signal.cc index a67df458c4..13946bd14b 100644 --- a/libc/signal.cc +++ b/libc/signal.cc @@ -21,6 +21,10 @@ #include #include +#ifdef __x86_64__ +#include "tls-switch.hh" +#endif + using namespace osv::clock::literals; namespace osv { @@ -393,6 +397,11 @@ int kill(pid_t pid, int sig) signal_actions[sigidx].sa_flags = 0; signal_actions[sigidx].sa_handler = SIG_DFL; } +#ifdef __x86_64__ + //In case this signal handler thread has specified app thread local storage + //let us switch to it before invoking the user handler routine + arch::user_tls_switch tls_switch; +#endif if (sa.sa_flags & SA_SIGINFO) { // FIXME: proper second (siginfo) and third (context) arguments (See example in call_signal_handler) sa.sa_sigaction(sig, nullptr, nullptr); @@ -401,6 +410,26 @@ int kill(pid_t pid, int sig) } }, sched::thread::attr().detached().stack(65536).name("signal_handler"), false, true); + //If we are running statically linked executable or a dynamic one with Linux + //dynamic linker, its threads very likely use app thread local storage and signal + //routine may rely on it presence. For that reason we use app TLS of the current + //thread if it has one. Otherwise we find 1st app thread with non-null app TLS + //and assign the signal handler thread app TLS to it so it is switched to (see above). + //TODO: Ideally we should only run the logic below if the current app is statically + //linked executable or a dynamic one ran with Linux dynamic linker + //(what if we are handling "Ctrl-C"?) + u64 app_tcb = sched::thread::current()->get_app_tcb(); + if (!app_tcb) { + auto first_app_thread = sched::find_first_app_thread([&](sched::thread &t) { + return t.get_app_tcb(); + }); + if (first_app_thread) { + app_tcb = first_app_thread->get_app_tcb(); + } + } + if (app_tcb) { + t->set_app_tcb(app_tcb); + } t->start(); } return 0;