Skip to content

Commit

Permalink
Reset the signal handlers at program exit
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cgzones authored and BenBE committed Aug 16, 2021
1 parent b42c441 commit 68460b2
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 15 deletions.
48 changes: 33 additions & 15 deletions CRT.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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())
Expand Down
2 changes: 2 additions & 0 deletions CRT.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions CommandLine.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 68460b2

Please sign in to comment.