From 137db219a7266300ffde1aa75d781284fb0807cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 25 Mar 2024 20:28:21 +0100 Subject: [PATCH 1/4] gmain: Use alternate signal stack if the application provides one Some applications, toolkits or languages may define an alternative stack to use for traces. This is for example the case of go. So, in case an application defines an alternate signal stack, GLib should use that instead of the default one to receive signals otherwise it may break the application expectations and write where it's not allowed to. --- glib/gmain.c | 3 +++ glib/tests/unix.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/glib/gmain.c b/glib/gmain.c index ded5dccbf..329e6d399 100644 --- a/glib/gmain.c +++ b/glib/gmain.c @@ -5670,6 +5670,9 @@ ref_unix_signal_handler_unlocked (int signum) action.sa_flags = SA_RESTART | SA_NOCLDSTOP; #else action.sa_flags = SA_NOCLDSTOP; +#endif +#ifdef SA_ONSTACK + action.sa_flags |= SA_ONSTACK; #endif sigaction (signum, &action, NULL); } diff --git a/glib/tests/unix.c b/glib/tests/unix.c index 1defb3de0..08c36c929 100644 --- a/glib/tests/unix.c +++ b/glib/tests/unix.c @@ -30,6 +30,7 @@ #include "glib-unix.h" #include "gstdio.h" +#include #include #include #include @@ -507,6 +508,7 @@ on_sig_received_2 (gpointer data) static void test_signal (int signum) { + struct sigaction action; GMainLoop *mainloop; int id; @@ -515,6 +517,17 @@ test_signal (int signum) sig_received = FALSE; sig_counter = 0; g_unix_signal_add (signum, on_sig_received, mainloop); + + g_assert_no_errno (sigaction (signum, NULL, &action)); + + g_assert_true (action.sa_flags & SA_NOCLDSTOP); +#ifdef SA_RESTART + g_assert_true (action.sa_flags & SA_RESTART); +#endif +#ifdef SA_ONSTACK + g_assert_true (action.sa_flags & SA_ONSTACK); +#endif + kill (getpid (), signum); g_assert (!sig_received); id = g_timeout_add (5000, on_sig_timeout, mainloop); @@ -554,6 +567,46 @@ test_sigterm (void) test_signal (SIGTERM); } +static void +test_signal_alternate_stack (int signal) +{ +#ifndef SA_ONSTACK + g_test_skip ("alternate stack is not supported"); +#else + guint8 stack_memory[MINSIGSTKSZ]; + guint8 zero_mem[MINSIGSTKSZ]; + stack_t stack = { .ss_sp = stack_memory, .ss_size = MINSIGSTKSZ }; + stack_t old_stack = { 0 }; + + memset (stack_memory, 0, MINSIGSTKSZ); + memset (zero_mem, 0, MINSIGSTKSZ); + g_assert_cmpmem (stack_memory, MINSIGSTKSZ, zero_mem, MINSIGSTKSZ); + + g_assert_no_errno (sigaltstack (&stack, &old_stack)); + + test_signal (signal); + + /* Very stupid check to ensure that the alternate stack is used instead of + * the default one. This test would fail if SA_ONSTACK wouldn't be set. + */ + g_assert_cmpint (memcmp (stack_memory, zero_mem, MINSIGSTKSZ), !=, 0); + + g_assert_no_errno (sigaltstack (&old_stack, NULL)); +#endif +} + +static void +test_sighup_alternate_stack (void) +{ + test_signal_alternate_stack (SIGHUP); +} + +static void +test_sigterm_alternate_stack (void) +{ + test_signal_alternate_stack (SIGTERM); +} + static void test_sighup_add_remove (void) { @@ -807,6 +860,9 @@ main (int argc, g_test_add_func ("/glib-unix/sighup", test_sighup); g_test_add_func ("/glib-unix/sigterm", test_sigterm); g_test_add_func ("/glib-unix/sighup_again", test_sighup); + g_test_add_func ("/glib-unix/sighup/alternate-stack", test_sighup_alternate_stack); + g_test_add_func ("/glib-unix/sigterm/alternate-stack", test_sigterm_alternate_stack); + g_test_add_func ("/glib-unix/sighup_again/alternate-stack", test_sighup_alternate_stack); g_test_add_func ("/glib-unix/sighup_add_remove", test_sighup_add_remove); g_test_add_func ("/glib-unix/sighup_nested", test_sighup_nested); g_test_add_func ("/glib-unix/callback_after_signal", test_callback_after_signal); From 8c842792a9c8dd0b98034e14e4a3dab9d0115ed5 Mon Sep 17 00:00:00 2001 From: John Ralls Date: Mon, 15 Apr 2024 17:12:28 +0200 Subject: [PATCH 2/4] glib/tests/unix: Only check for SA_NOCLDSTOP on SIGCHLD --- glib/tests/unix.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/glib/tests/unix.c b/glib/tests/unix.c index 08c36c929..d83c2e1d4 100644 --- a/glib/tests/unix.c +++ b/glib/tests/unix.c @@ -520,7 +520,9 @@ test_signal (int signum) g_assert_no_errno (sigaction (signum, NULL, &action)); - g_assert_true (action.sa_flags & SA_NOCLDSTOP); + if (signum == SIGCHLD) + g_assert_true (action.sa_flags & SA_NOCLDSTOP); + #ifdef SA_RESTART g_assert_true (action.sa_flags & SA_RESTART); #endif From 035c3183248eced7d52bc2a76a4ee76c837ecd1d Mon Sep 17 00:00:00 2001 From: John Ralls Date: Mon, 15 Apr 2024 17:12:57 +0200 Subject: [PATCH 3/4] glib/tests/unix: Disable the alternate signal stack using SS_DISABLE In other unix implementations other than linux, sigaltstack can't use a NULL pointer for old_stack, so let's use SS_DISABLE instead to disable the alternate stack. Co-Authored-By: Marco Trevisan --- glib/tests/unix.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/glib/tests/unix.c b/glib/tests/unix.c index d83c2e1d4..6f9469cce 100644 --- a/glib/tests/unix.c +++ b/glib/tests/unix.c @@ -593,7 +593,14 @@ test_signal_alternate_stack (int signal) */ g_assert_cmpint (memcmp (stack_memory, zero_mem, MINSIGSTKSZ), !=, 0); - g_assert_no_errno (sigaltstack (&old_stack, NULL)); + memset (stack_memory, 0, MINSIGSTKSZ); + g_assert_cmpmem (stack_memory, MINSIGSTKSZ, zero_mem, MINSIGSTKSZ); + + stack.ss_flags = SS_DISABLE; + g_assert_no_errno (sigaltstack (&stack, &old_stack)); + + test_signal (signal); + g_assert_cmpmem (stack_memory, MINSIGSTKSZ, zero_mem, MINSIGSTKSZ); #endif } From 3d474bd8c153d57233289ad4cb9615798c3c3c05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 16 Apr 2024 12:05:52 +0200 Subject: [PATCH 4/4] unix: Prevent compiler optimization to ignore our memset to zero It's well known that memset may be optimized out by compilers and this is one of these cases that freebsd CI highlighted. To prevent this to happen we should use memset_explicit() but that's C23, so till we don't support that, let's re-implement that ourself making the compiler not to optimize our memset's. In theory we could just rely on C11's memset_s, but that's not working either in freebsd. --- glib/tests/unix.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/glib/tests/unix.c b/glib/tests/unix.c index 6f9469cce..296a589e6 100644 --- a/glib/tests/unix.c +++ b/glib/tests/unix.c @@ -593,6 +593,10 @@ test_signal_alternate_stack (int signal) */ g_assert_cmpint (memcmp (stack_memory, zero_mem, MINSIGSTKSZ), !=, 0); + /* We need to memset again zero_mem since compiler may have optimized it out + * as we've seen in freebsd CI. + */ + memset (zero_mem, 0, MINSIGSTKSZ); memset (stack_memory, 0, MINSIGSTKSZ); g_assert_cmpmem (stack_memory, MINSIGSTKSZ, zero_mem, MINSIGSTKSZ);