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
This commit is contained in:
Christian Göttsche 2021-08-14 19:52:26 +02:00 committed by BenBE
parent b42c441ee0
commit 68460b25e3
3 changed files with 37 additions and 15 deletions

48
CRT.c
View File

@ -817,6 +817,38 @@ static void dumpStderr(void) {
static struct sigaction old_sig_handler[32]; 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) { void CRT_init(const Settings* settings, bool allowUnicode) {
redirectStderr(); redirectStderr();
@ -875,21 +907,7 @@ void CRT_init(const Settings* settings, bool allowUnicode) {
} }
} }
struct sigaction act; CRT_installSignalHandlers();
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);
use_default_colors(); use_default_colors();
if (!has_colors()) if (!has_colors())

2
CRT.h
View File

@ -176,6 +176,8 @@ void CRT_init(const Settings* settings, bool allowUnicode);
void CRT_done(void); void CRT_done(void);
void CRT_resetSignalHandlers(void);
int CRT_readKey(void); int CRT_readKey(void);
void CRT_disableDelay(void); void CRT_disableDelay(void);

View File

@ -389,6 +389,8 @@ int CommandLine_run(const char* name, int argc, char** argv) {
if (flags.pidMatchList) if (flags.pidMatchList)
Hashtable_delete(flags.pidMatchList); Hashtable_delete(flags.pidMatchList);
CRT_resetSignalHandlers();
/* Delete these last, since they can get accessed in the crash handler */ /* Delete these last, since they can get accessed in the crash handler */
Settings_delete(settings); Settings_delete(settings);
if (dc) if (dc)