From c53cd79e6883d247d077f25cee72a5e608ae4b5d Mon Sep 17 00:00:00 2001 From: Martin Mares Date: Mon, 11 Dec 2006 23:45:56 +0100 Subject: [PATCH] Fixed thread safety of signal handling. Signals delivery is generally not deterministic in multi-threaded programs, but SIGSEGV is always sent by the kernel to the thread which caused the fault. Replaced the global signal handler array by a pointer in the per-thread context and replaced saving of original handles (which was wrong even in some non-threaded cases) by locked use counters. Also moved logging of the SIGSEGV error message out of the signal handler to be sure there are no clashes. When libucw is compiled without threading, everything is nearly as simple as before, because the thread context struct is static and all locks disappear. --- lib/lib.h | 10 ++++----- lib/lizard-safe.c | 15 ++++++------- lib/sighandler.c | 56 +++++++++++++++++++++++++++++++++-------------- 3 files changed, 50 insertions(+), 31 deletions(-) diff --git a/lib/lib.h b/lib/lib.h index 43135c6c..4a6b9d81 100644 --- a/lib/lib.h +++ b/lib/lib.h @@ -259,13 +259,11 @@ void sync_dir(byte *name); /* sighandler.c */ -typedef int (*sh_sighandler_t)(int); - /* obtains signum, returns nonzero if abort() should be called */ -extern sh_sighandler_t signal_handler[]; +typedef int (*sh_sighandler_t)(int); // gets signum, returns nonzero if abort() should be called -struct sigaction; -void handle_signal(int signum, struct sigaction *oldact); -void unhandle_signal(int signum, struct sigaction *oldact); +void handle_signal(int signum); +void unhandle_signal(int signum); +sh_sighandler_t set_signal_handler(int signum, sh_sighandler_t new); /* string.c */ diff --git a/lib/lizard-safe.c b/lib/lizard-safe.c index 84e52da1..7553b459 100644 --- a/lib/lizard-safe.c +++ b/lib/lizard-safe.c @@ -8,6 +8,7 @@ */ #include "lib/lib.h" +#include "lib/threads.h" #include "lib/lizard.h" #include @@ -20,7 +21,6 @@ struct lizard_buffer { uns len; void *ptr; - struct sigaction old_sigsegv_handler; }; struct lizard_buffer * @@ -29,16 +29,16 @@ lizard_alloc(void) struct lizard_buffer *buf = xmalloc(sizeof(struct lizard_buffer)); buf->len = 0; buf->ptr = NULL; - handle_signal(SIGSEGV, &buf->old_sigsegv_handler); + handle_signal(SIGSEGV); return buf; } void lizard_free(struct lizard_buffer *buf) { + unhandle_signal(SIGSEGV); if (buf->ptr) munmap(buf->ptr, buf->len + PAGE_SIZE); - unhandle_signal(SIGSEGV, &buf->old_sigsegv_handler); xfree(buf); } @@ -56,7 +56,7 @@ lizard_realloc(struct lizard_buffer *buf, uns max_len) buf->len = max_len; buf->ptr = mmap(NULL, buf->len + PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); if (buf->ptr == MAP_FAILED) - die("mmap(anonymous): %m"); + die("mmap(anonymous, %d bytes): %m", (uns)(buf->len + PAGE_SIZE)); if (mprotect(buf->ptr + buf->len, PAGE_SIZE, PROT_NONE) < 0) die("mprotect: %m"); } @@ -65,7 +65,6 @@ static jmp_buf safe_decompress_jump; static int sigsegv_handler(int signal UNUSED) { - log(L_ERROR, "SIGSEGV caught in lizard_decompress()"); longjmp(safe_decompress_jump, 1); return 1; } @@ -81,8 +80,7 @@ lizard_decompress_safe(byte *in, struct lizard_buffer *buf, uns expected_length) uns lock_offset = ALIGN_TO(expected_length + 3, PAGE_SIZE); // +3 due to the unaligned access if (lock_offset > buf->len) lizard_realloc(buf, lock_offset); - volatile sh_sighandler_t old_handler = signal_handler[SIGSEGV]; - signal_handler[SIGSEGV] = sigsegv_handler; + volatile sh_sighandler_t old_handler = set_signal_handler(SIGSEGV, sigsegv_handler); byte *ptr; if (!setjmp(safe_decompress_jump)) { @@ -96,9 +94,10 @@ lizard_decompress_safe(byte *in, struct lizard_buffer *buf, uns expected_length) } else { + log(L_ERROR, "SIGSEGV caught in lizard_decompress()"); ptr = NULL; errno = EFAULT; } - signal_handler[SIGSEGV] = old_handler; + set_signal_handler(SIGSEGV, old_handler); return ptr; } diff --git a/lib/sighandler.c b/lib/sighandler.c index 4d991d85..4991f9a3 100644 --- a/lib/sighandler.c +++ b/lib/sighandler.c @@ -2,41 +2,63 @@ * UCW Library -- Catching of signals and calling callback functions * * (c) 2004, Robert Spalek + * (c) 2006 Martin Mares */ #include "lib/lib.h" +#include "lib/threads.h" #include #include #include -sh_sighandler_t signal_handler[NSIG]; +static int sig_handler_nest[NSIG]; +static struct sigaction sig_handler_old[NSIG]; static void signal_handler_internal(int sig) { - if (signal_handler[sig]) - { - if (!signal_handler[sig](sig)) - return; - } - abort(); + struct ucwlib_context *ctx = ucwlib_thread_context(); + if (!ctx->signal_handlers[sig] || ctx->signal_handlers[sig](sig)) + abort(); } void -handle_signal(int signum, struct sigaction *oldact) +handle_signal(int signum) { - struct sigaction act; - bzero(&act, sizeof(act)); - act.sa_handler = signal_handler_internal; - act.sa_flags = SA_NODEFER; - if (sigaction(signum, &act, oldact) < 0) - die("sigaction: %m"); + ucwlib_lock(); + if (!sig_handler_nest[signum]++) + { + struct sigaction act; + bzero(&act, sizeof(act)); + act.sa_handler = signal_handler_internal; + act.sa_flags = SA_NODEFER; + if (sigaction(signum, &act, &sig_handler_old[signum]) < 0) + die("sigaction: %m"); + } + ucwlib_unlock(); } void -unhandle_signal(int signum, struct sigaction *oldact) +unhandle_signal(int signum) { - if (sigaction(signum, oldact, NULL) < 0) - die("sigaction: %m"); + ucwlib_lock(); + ASSERT(sig_handler_nest[signum]); + if (!--sig_handler_nest[signum]) + { + if (sigaction(signum, &sig_handler_old[signum], NULL) < 0) + die("sigaction: %m"); + } + ucwlib_unlock(); +} + +sh_sighandler_t +set_signal_handler(int signum, sh_sighandler_t new) +{ + struct ucwlib_context *ctx = ucwlib_thread_context(); + if (!ctx->signal_handlers) + ctx->signal_handlers = xmalloc_zero(NSIG * sizeof(sh_sighandler_t)); + sh_sighandler_t old = ctx->signal_handlers[signum]; + ctx->signal_handlers[signum] = new; + return old; } -- 2.39.2