From 68460b25e39c583d04edaac3ea1ce3e5bdee6b89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Sat, 14 Aug 2021 19:52:26 +0200 Subject: [PATCH] Reset the signal handlers at program exit The signal handler will access the Settings struct, which gets freed at normal program finalization. When using leak sanitizers with ASAN_OPTIONS=abort_on_error=1, which runs after program termination, any leak causes SIGABRT to be raised, calling the crash handler, which will derefernce the freed Settings. ==44741==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000000080 at pc 0x0000005680df bp 0x7fffe335e960 sp 0x7fffe335e958 READ of size 8 at 0x60d000000080 thread T0 #0 0x5680de in Settings_write /home/christian/Coding/workspaces/htop/Settings.c:329:26 #1 0x4f77b7 in CRT_handleSIGSEGV /home/christian/Coding/workspaces/htop/CRT.c:1020:4 #2 0x7f8a1120c13f (/lib/x86_64-linux-gnu/libpthread.so.0+0x1413f) #3 0x7f8a11042ce0 in __libc_signal_restore_set signal/../sysdeps/unix/sysv/linux/internal-signals.h:86:3 #4 0x7f8a11042ce0 in raise signal/../sysdeps/unix/sysv/linux/raise.c:48:3 #5 0x7f8a1102c536 in abort stdlib/abort.c:79:7 #6 0x4c3db6 in __sanitizer::Abort() (/home/christian/Coding/workspaces/htop/htop+0x4c3db6) #7 0x4c2090 in __sanitizer::Die() (/home/christian/Coding/workspaces/htop/htop+0x4c2090) #8 0x4d0a17 in __lsan::HandleLeaks() (/home/christian/Coding/workspaces/htop/htop+0x4d0a17) #9 0x4cd950 in __lsan::DoLeakCheck() (/home/christian/Coding/workspaces/htop/htop+0x4cd950) #10 0x7f8a110454d6 in __run_exit_handlers stdlib/exit.c:108:8 #11 0x7f8a11045679 in exit stdlib/exit.c:139:3 #12 0x7f8a1102dd10 in __libc_start_main csu/../csu/libc-start.c:342:3 #13 0x428a19 in _start (/home/christian/Coding/workspaces/htop/htop+0x428a19) 0x60d000000080 is located 64 bytes inside of 144-byte region [0x60d000000040,0x60d0000000d0) freed by thread T0 here: #0 0x4a4f72 in free (/home/christian/Coding/workspaces/htop/htop+0x4a4f72) #1 0x566693 in Settings_delete /home/christian/Coding/workspaces/htop/Settings.c:32:4 #2 0x4ede10 in CommandLine_run /home/christian/Coding/workspaces/htop/CommandLine.c:393:4 #3 0x4d6f32 in main /home/christian/Coding/workspaces/htop/htop.c:15:11 #4 0x7f8a1102dd09 in __libc_start_main csu/../csu/libc-start.c:308:16 previously allocated by thread T0 here: #0 0x4a5372 in __interceptor_calloc (/home/christian/Coding/workspaces/htop/htop+0x4a5372) #1 0x57f61a in xCalloc /home/christian/Coding/workspaces/htop/XUtils.c:55:17 #2 0x5688a6 in Settings_new /home/christian/Coding/workspaces/htop/Settings.c:392:21 #3 0x4ecb57 in CommandLine_run /home/christian/Coding/workspaces/htop/CommandLine.c:303:25 #4 0x4d6f32 in main /home/christian/Coding/workspaces/htop/htop.c:15:11 #5 0x7f8a1102dd09 in __libc_start_main csu/../csu/libc-start.c:308:16 SUMMARY: AddressSanitizer: heap-use-after-free /home/christian/Coding/workspaces/htop/Settings.c:329:26 in Settings_write --- CRT.c | 48 +++++++++++++++++++++++++++++++++--------------- CRT.h | 2 ++ CommandLine.c | 2 ++ 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/CRT.c b/CRT.c index 6a87882e..ed78b59e 100644 --- a/CRT.c +++ b/CRT.c @@ -817,6 +817,38 @@ static void dumpStderr(void) { static struct sigaction old_sig_handler[32]; +static void CRT_installSignalHandlers(void) { + struct sigaction act; + sigemptyset (&act.sa_mask); + act.sa_flags = (int)SA_RESETHAND | SA_NODEFER; + act.sa_handler = CRT_handleSIGSEGV; + sigaction (SIGSEGV, &act, &old_sig_handler[SIGSEGV]); + sigaction (SIGFPE, &act, &old_sig_handler[SIGFPE]); + sigaction (SIGILL, &act, &old_sig_handler[SIGILL]); + sigaction (SIGBUS, &act, &old_sig_handler[SIGBUS]); + sigaction (SIGPIPE, &act, &old_sig_handler[SIGPIPE]); + sigaction (SIGSYS, &act, &old_sig_handler[SIGSYS]); + sigaction (SIGABRT, &act, &old_sig_handler[SIGABRT]); + + signal(SIGINT, CRT_handleSIGTERM); + signal(SIGTERM, CRT_handleSIGTERM); + signal(SIGQUIT, CRT_handleSIGTERM); +} + +void CRT_resetSignalHandlers(void) { + sigaction (SIGSEGV, &old_sig_handler[SIGSEGV], NULL); + sigaction (SIGFPE, &old_sig_handler[SIGFPE], NULL); + sigaction (SIGILL, &old_sig_handler[SIGILL], NULL); + sigaction (SIGBUS, &old_sig_handler[SIGBUS], NULL); + sigaction (SIGPIPE, &old_sig_handler[SIGPIPE], NULL); + sigaction (SIGSYS, &old_sig_handler[SIGSYS], NULL); + sigaction (SIGABRT, &old_sig_handler[SIGABRT], NULL); + + signal(SIGINT, SIG_DFL); + signal(SIGTERM, SIG_DFL); + signal(SIGQUIT, SIG_DFL); +} + void CRT_init(const Settings* settings, bool allowUnicode) { redirectStderr(); @@ -875,21 +907,7 @@ void CRT_init(const Settings* settings, bool allowUnicode) { } } - struct sigaction act; - sigemptyset (&act.sa_mask); - act.sa_flags = (int)SA_RESETHAND | SA_NODEFER; - act.sa_handler = CRT_handleSIGSEGV; - sigaction (SIGSEGV, &act, &old_sig_handler[SIGSEGV]); - sigaction (SIGFPE, &act, &old_sig_handler[SIGFPE]); - sigaction (SIGILL, &act, &old_sig_handler[SIGILL]); - sigaction (SIGBUS, &act, &old_sig_handler[SIGBUS]); - sigaction (SIGPIPE, &act, &old_sig_handler[SIGPIPE]); - sigaction (SIGSYS, &act, &old_sig_handler[SIGSYS]); - sigaction (SIGABRT, &act, &old_sig_handler[SIGABRT]); - - signal(SIGINT, CRT_handleSIGTERM); - signal(SIGTERM, CRT_handleSIGTERM); - signal(SIGQUIT, CRT_handleSIGTERM); + CRT_installSignalHandlers(); use_default_colors(); if (!has_colors()) diff --git a/CRT.h b/CRT.h index 9f3ecb44..4e6ff672 100644 --- a/CRT.h +++ b/CRT.h @@ -176,6 +176,8 @@ void CRT_init(const Settings* settings, bool allowUnicode); void CRT_done(void); +void CRT_resetSignalHandlers(void); + int CRT_readKey(void); void CRT_disableDelay(void); diff --git a/CommandLine.c b/CommandLine.c index b4144d8d..ecfc731e 100644 --- a/CommandLine.c +++ b/CommandLine.c @@ -389,6 +389,8 @@ int CommandLine_run(const char* name, int argc, char** argv) { if (flags.pidMatchList) Hashtable_delete(flags.pidMatchList); + CRT_resetSignalHandlers(); + /* Delete these last, since they can get accessed in the crash handler */ Settings_delete(settings); if (dc)