signals: switch to app TLS if available#1288
Open
wkozaczuk wants to merge 1 commit intocloudius-systems:masterfrom
Open
signals: switch to app TLS if available#1288wkozaczuk wants to merge 1 commit intocloudius-systems:masterfrom
wkozaczuk wants to merge 1 commit intocloudius-systems:masterfrom
Conversation
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 <jwkozaczuk@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: This PR is a proposal and I am not sure we should merge it. Even though it fixes #1278 I have found that Golang apps signal handlers seem to validate that the stack used by signal handler routine was created by Golang. This means we may re-think how we handle user signal routine and possibly make it be executed on one of the application's threads (just like Linux does) instead of creating a new thread.
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 dynamic ones with Linux dynamic linker, the application threads create TLS on their own, and user signal handler may reference thread-local variables that do not exist in the TLS of the new signal handler thread. As a result, the app would crash like so:
This patch applies a bit of trickery (potentially dangerous if user handler tries to write to TLS) and selects 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:
Refers #1278