[pacman-dev] [PATCH 1/2] sighandler: block signals while handling SIGSEGV
If we get SIGSEGV we need to bail out quickly, leaving other signals unblocked could lead to other signal handlers getting triggered. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- Signals are hard. I'd appreciate if somebody could double check my math on these patches. src/pacman/sighandler.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c index ebcdebae..a4849a0c 100644 --- a/src/pacman/sighandler.c +++ b/src/pacman/sighandler.c @@ -96,7 +96,7 @@ void install_segv_handler(void) { struct sigaction new_action; new_action.sa_handler = segv_handler; - sigemptyset(&new_action.sa_mask); + sigfillset(&new_action.sa_mask); new_action.sa_flags = SA_RESTART; sigaction(SIGSEGV, &new_action, NULL); } -- 2.21.0
Overriding the segfault handler prevents the creation of core dumps by the default handler, which makes debugging segfaults difficult. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/sighandler.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c index a4849a0c..082756b5 100644 --- a/src/pacman/sighandler.c +++ b/src/pacman/sighandler.c @@ -38,6 +38,15 @@ static ssize_t xwrite(int fd, const void *buf, size_t count) return ret; } +static void _reset_handler(int signum) +{ + struct sigaction new_action; + sigemptyset(&new_action.sa_mask); + new_action.sa_handler = SIG_DFL; + new_action.sa_flags = 0; + sigaction(signum, &new_action, NULL); +} + /** Catches thrown signals. Performs necessary cleanup to ensure database is * in a consistent state. * @param signum the thrown signal @@ -76,19 +85,26 @@ void install_soft_interrupt_handler(void) void remove_soft_interrupt_handler(void) { - struct sigaction new_action; - sigemptyset(&new_action.sa_mask); - new_action.sa_handler = SIG_DFL; - new_action.sa_flags = 0; - sigaction(SIGINT, &new_action, NULL); - sigaction(SIGHUP, &new_action, NULL); + _reset_handler(SIGINT); + _reset_handler(SIGHUP); } static void segv_handler(int signum) { + sigset_t segvset; const char msg[] = "\nerror: segmentation fault\n" "Please submit a full bug report with --debug if appropriate.\n"; xwrite(STDERR_FILENO, msg, sizeof(msg) - 1); + + /* restore the default handler */ + _reset_handler(signum); + /* unblock SIGSEGV */ + sigaddset(&segvset, signum); + sigprocmask(SIG_UNBLOCK, &segvset, NULL); + /* re-raise to trigger a core dump */ + raise(signum); + + /* raise should immediately abort, but just to make absolutely sure */ _Exit(signum); } -- 2.21.0
On 8/6/19 2:12 pm, Andrew Gregory wrote:
If we get SIGSEGV we need to bail out quickly, leaving other signals unblocked could lead to other signal handlers getting triggered.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
Signals are hard. I'd appreciate if somebody could double check my math on these patches.
This is good. Allan
src/pacman/sighandler.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c index ebcdebae..a4849a0c 100644 --- a/src/pacman/sighandler.c +++ b/src/pacman/sighandler.c @@ -96,7 +96,7 @@ void install_segv_handler(void) { struct sigaction new_action; new_action.sa_handler = segv_handler; - sigemptyset(&new_action.sa_mask); + sigfillset(&new_action.sa_mask); new_action.sa_flags = SA_RESTART; sigaction(SIGSEGV, &new_action, NULL); }
On 8/6/19 2:12 pm, Andrew Gregory wrote:
Overriding the segfault handler prevents the creation of core dumps by the default handler, which makes debugging segfaults difficult.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
This looks good to me. I did a double take at the underscore leading the function name, but it turns out we use it elsewhere in pacman.
src/pacman/sighandler.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c index a4849a0c..082756b5 100644 --- a/src/pacman/sighandler.c +++ b/src/pacman/sighandler.c @@ -38,6 +38,15 @@ static ssize_t xwrite(int fd, const void *buf, size_t count) return ret; }
+static void _reset_handler(int signum) +{ + struct sigaction new_action; + sigemptyset(&new_action.sa_mask); + new_action.sa_handler = SIG_DFL; + new_action.sa_flags = 0; + sigaction(signum, &new_action, NULL); +} + /** Catches thrown signals. Performs necessary cleanup to ensure database is * in a consistent state. * @param signum the thrown signal @@ -76,19 +85,26 @@ void install_soft_interrupt_handler(void)
void remove_soft_interrupt_handler(void) { - struct sigaction new_action; - sigemptyset(&new_action.sa_mask); - new_action.sa_handler = SIG_DFL; - new_action.sa_flags = 0; - sigaction(SIGINT, &new_action, NULL); - sigaction(SIGHUP, &new_action, NULL); + _reset_handler(SIGINT); + _reset_handler(SIGHUP); }
static void segv_handler(int signum) { + sigset_t segvset; const char msg[] = "\nerror: segmentation fault\n" "Please submit a full bug report with --debug if appropriate.\n"; xwrite(STDERR_FILENO, msg, sizeof(msg) - 1); + + /* restore the default handler */ + _reset_handler(signum); + /* unblock SIGSEGV */ + sigaddset(&segvset, signum); + sigprocmask(SIG_UNBLOCK, &segvset, NULL); + /* re-raise to trigger a core dump */ + raise(signum); + + /* raise should immediately abort, but just to make absolutely sure */ _Exit(signum); }
participants (2)
-
Allan McRae
-
Andrew Gregory