Skip to content

Commit

Permalink
signals: switch to app TLS if available
Browse files Browse the repository at this point in the history
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 <signal handler called>
11 __GI___pthread_cleanup_upto (target=target@entry=0x200000205080 <sigend_jmp_buf>, targetframe=targetframe@entry=0x40007fe93fc8 "\005\"D\177") at ./nptl/pthread_cleanup_upto.c:32
12 0x000020007f44232c in _longjmp_unwind (env=env@entry=0x200000205080 <sigend_jmp_buf>, val=val@entry=1) at ../sysdeps/nptl/jmp-unwind.c:27
13 0x000020007f442205 in __libc_siglongjmp (env=0x200000205080 <sigend_jmp_buf>, 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 cloudius-systems#1278

Signed-off-by: Waldemar Kozaczuk <[email protected]>
  • Loading branch information
wkozaczuk committed Dec 18, 2023
1 parent ad355ab commit cc976d1
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 0 deletions.
22 changes: 22 additions & 0 deletions arch/x64/tls-switch.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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<u64>(_kernel_tcb->app_tcb));
}
}

~user_tls_switch() {
//Switch to kernel tcb if app tcb present
if (_kernel_tcb->app_tcb) {
set_fsbase(reinterpret_cast<u64>(_kernel_tcb->self));
}
}
};

}

Expand Down
10 changes: 10 additions & 0 deletions core/sched.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2088,6 +2088,16 @@ void with_thread_by_id(unsigned id, std::function<void(thread *)> f) {
}
}

thread *find_first_app_thread(std::function<bool(thread &)> f) {
WITH_LOCK(thread_map_mutex) {
for (auto th : thread_map) {
if(th.second->is_app() && f(*th.second)) {
return th.second;
}
}
}
return nullptr;
}

}

Expand Down
2 changes: 2 additions & 0 deletions include/osv/sched.hh
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,8 @@ void with_all_threads(std::function<void(sched::thread &)>);
// should return quickly.
void with_thread_by_id(unsigned id, std::function<void(sched::thread *)>);

thread *find_first_app_thread(std::function<bool(thread &)> f);

}

#endif /* SCHED_HH_ */
29 changes: 29 additions & 0 deletions libc/signal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
#include <osv/pid.h>
#include <osv/export.h>

#ifdef __x86_64__
#include "tls-switch.hh"
#endif

using namespace osv::clock::literals;

namespace osv {
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down

0 comments on commit cc976d1

Please sign in to comment.