rpms
/
ocaml
2
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity
ocaml/0018-More-prudent-deallocat...

195 lines
6.8 KiB
Diff
Raw Permalink Blame History

From 1c4df0352d5f75464acdb27377ccd10d1e0ba10a Mon Sep 17 00:00:00 2001
From: Xavier Leroy <xavierleroy@users.noreply.github.com>
Date: Mon, 22 Aug 2022 16:06:50 +0200
Subject: [PATCH 18/24] More prudent deallocation of alternate signal stack
(#11496)
After `caml_setup_stack_overflow_detection` is called,
a C library can install its own alternate stack for signal handling.
Therefore, `caml_stop_stack_overflow_detection` must not free the
alternate signal stack block, only the block that
`caml_setup_stack_overflow_detection` allocated.
Fixes: #11489
---
Changes | 4 +++
otherlibs/systhreads/st_stubs.c | 5 ++--
runtime/caml/signals.h | 4 +--
runtime/signals_byt.c | 4 +--
runtime/signals_nat.c | 47 +++++++++++++++++++++------------
5 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/Changes b/Changes
index 6b537edca9..4d1bb10435 100644
--- a/Changes
+++ b/Changes
@@ -41,6 +41,10 @@ OCaml 4.14 maintenance branch
mingw-w64 i686 port.
(David Allsopp, review by Xavier Leroy and Sébastien Hinderer)
+- #11489, #11496: More prudent deallocation of alternate signal stack
+ (Xavier Leroy, report by @rajdakin, review by Florian Angeletti)
+
+
OCaml 4.14.0 (28 March 2022)
----------------------------
diff --git a/otherlibs/systhreads/st_stubs.c b/otherlibs/systhreads/st_stubs.c
index b7a6a9a6bb..043e07031e 100644
--- a/otherlibs/systhreads/st_stubs.c
+++ b/otherlibs/systhreads/st_stubs.c
@@ -524,6 +524,7 @@ static ST_THREAD_FUNCTION caml_thread_start(void * arg)
{
caml_thread_t th = (caml_thread_t) arg;
value clos;
+ void * signal_stack;
#ifdef NATIVE_CODE
struct longjmp_buffer termination_buf;
char tos;
@@ -536,7 +537,7 @@ static ST_THREAD_FUNCTION caml_thread_start(void * arg)
/* Acquire the global mutex */
caml_leave_blocking_section();
st_thread_set_id(Ident(th->descr));
- caml_setup_stack_overflow_detection();
+ signal_stack = caml_setup_stack_overflow_detection();
#ifdef NATIVE_CODE
/* Setup termination handler (for caml_thread_exit) */
if (sigsetjmp(termination_buf.buf, 0) == 0) {
@@ -550,7 +551,7 @@ static ST_THREAD_FUNCTION caml_thread_start(void * arg)
#ifdef NATIVE_CODE
}
#endif
- caml_stop_stack_overflow_detection();
+ caml_stop_stack_overflow_detection(signal_stack);
/* The thread now stops running */
return 0;
}
diff --git a/runtime/caml/signals.h b/runtime/caml/signals.h
index c6aeebfc78..62b0e7fafa 100644
--- a/runtime/caml/signals.h
+++ b/runtime/caml/signals.h
@@ -87,8 +87,8 @@ value caml_do_pending_actions_exn (void);
value caml_process_pending_actions_with_root (value extra_root); // raises
value caml_process_pending_actions_with_root_exn (value extra_root);
int caml_set_signal_action(int signo, int action);
-CAMLextern int caml_setup_stack_overflow_detection(void);
-CAMLextern int caml_stop_stack_overflow_detection(void);
+CAMLextern void * caml_setup_stack_overflow_detection(void);
+CAMLextern int caml_stop_stack_overflow_detection(void *);
CAMLextern void caml_init_signals(void);
CAMLextern void caml_terminate_signals(void);
CAMLextern void (*caml_enter_blocking_section_hook)(void);
diff --git a/runtime/signals_byt.c b/runtime/signals_byt.c
index 439fb56404..7cb461ac4d 100644
--- a/runtime/signals_byt.c
+++ b/runtime/signals_byt.c
@@ -81,7 +81,7 @@ int caml_set_signal_action(int signo, int action)
return 0;
}
-CAMLexport int caml_setup_stack_overflow_detection(void) { return 0; }
-CAMLexport int caml_stop_stack_overflow_detection(void) { return 0; }
+CAMLexport void * caml_setup_stack_overflow_detection(void) { return NULL; }
+CAMLexport int caml_stop_stack_overflow_detection(void * p) { return 0; }
CAMLexport void caml_init_signals(void) { }
CAMLexport void caml_terminate_signals(void) { }
diff --git a/runtime/signals_nat.c b/runtime/signals_nat.c
index 443f5d53b6..1dd8289c12 100644
--- a/runtime/signals_nat.c
+++ b/runtime/signals_nat.c
@@ -254,6 +254,10 @@ DECLARE_SIGNAL_HANDLER(segv_handler)
/* Initialization of signal stuff */
+#ifdef HAS_STACK_OVERFLOW_DETECTION
+static void * caml_signal_stack = NULL;
+#endif
+
void caml_init_signals(void)
{
/* Bound-check trap handling */
@@ -278,7 +282,8 @@ void caml_init_signals(void)
#endif
#ifdef HAS_STACK_OVERFLOW_DETECTION
- if (caml_setup_stack_overflow_detection() != -1) {
+ caml_signal_stack = caml_setup_stack_overflow_detection();
+ if (caml_signal_stack != NULL) {
struct sigaction act;
SET_SIGACT(act, segv_handler);
act.sa_flags |= SA_ONSTACK | SA_NODEFER;
@@ -314,7 +319,8 @@ void caml_terminate_signals(void)
#ifdef HAS_STACK_OVERFLOW_DETECTION
set_signal_default(SIGSEGV);
- caml_stop_stack_overflow_detection();
+ caml_stop_stack_overflow_detection(caml_signal_stack);
+ caml_signal_stack = NULL;
#endif
}
@@ -323,37 +329,44 @@ void caml_terminate_signals(void)
Each thread needs its own alternate stack.
The alternate stack used to be statically-allocated for the main thread,
but this is incompatible with Glibc 2.34 and newer, where SIGSTKSZ
- may not be a compile-time constant (issue #10250). */
+ may not be a compile-time constant (issue #10250).
+ Return the dynamically-allocated alternate signal stack, or NULL
+ if an error occurred.
+ The returned pointer must be passed to [caml_stop_stack_overflow_detection].
+*/
-CAMLexport int caml_setup_stack_overflow_detection(void)
+CAMLexport void * caml_setup_stack_overflow_detection(void)
{
#ifdef HAS_STACK_OVERFLOW_DETECTION
stack_t stk;
- stk.ss_sp = malloc(SIGSTKSZ);
- if (stk.ss_sp == NULL) return -1;
stk.ss_size = SIGSTKSZ;
+ stk.ss_sp = malloc(stk.ss_size);
+ if (stk.ss_sp == NULL) return NULL;
stk.ss_flags = 0;
if (sigaltstack(&stk, NULL) == -1) {
free(stk.ss_sp);
- return -1;
+ return NULL;
}
+ return stk.ss_sp;
+#else
+ return NULL;
#endif
- /* Success (or stack overflow detection not available) */
- return 0;
}
-CAMLexport int caml_stop_stack_overflow_detection(void)
+CAMLexport int caml_stop_stack_overflow_detection(void * signal_stack)
{
#ifdef HAS_STACK_OVERFLOW_DETECTION
stack_t oldstk, stk;
stk.ss_flags = SS_DISABLE;
+ stk.ss_sp = NULL; /* not required but avoids a valgrind false alarm */
+ stk.ss_size = SIGSTKSZ; /* macOS wants a valid size here */
if (sigaltstack(&stk, &oldstk) == -1) return -1;
- /* If caml_setup_stack_overflow_detection failed, we are not using
- an alternate signal stack. SS_DISABLE will be set in oldstk,
- and there is nothing to free in this case. */
- if (! (oldstk.ss_flags & SS_DISABLE)) free(oldstk.ss_sp);
- return 0;
-#else
- return 0;
+ /* Check whether someone else installed their own signal stack */
+ if (!(oldstk.ss_flags & SS_DISABLE) && oldstk.ss_sp != signal_stack) {
+ /* Re-activate their signal stack. */
+ sigaltstack(&oldstk, NULL);
+ }
+ free(signal_stack);
#endif
+ return 0;
}
--
2.37.0.rc2
Reference in New Issue View Git Blame Copy Permalink