Menu
▾
▴
[Valgrind-developers] [valgrind] Bug 492678 - SIGALRM race condition (sighandler called after timer disarmed
|
From: Paul F. <pa...@so...> - 2025-01-25 16:55:37
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=3a1498f13f1676a20ab80fc553fd4f6b94b3a347 commit 3a1498f13f1676a20ab80fc553fd4f6b94b3a347 Author: Paul Floyd <pj...@wa...> Date: Sat Jan 25 14:54:08 2025 +0100 Bug 492678 - SIGALRM race condition (sighandler called after timer disarmed Diff: --- .gitignore | 2 + coregrind/m_syswrap/syswrap-freebsd.c | 1 + coregrind/m_syswrap/syswrap-generic.c | 8 +++ coregrind/m_syswrap/syswrap-linux.c | 1 + coregrind/m_syswrap/syswrap-solaris.c | 1 + none/tests/Makefile.am | 6 +- none/tests/bug492678.c | 104 ++++++++++++++++++++++++++++++++++ none/tests/bug492678.stderr.exp | 0 none/tests/bug492678.vgtest | 3 + none/tests/timer_delete.c | 102 +++++++++++++++++++++++++++++++++ none/tests/timer_delete.stderr.exp | 0 11 files changed, 227 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index d44246b8bc..4a6874e5c1 100644 --- a/.gitignore +++ b/.gitignore @@ -1534,6 +1534,7 @@ /none/tests/bug129866 /none/tests/bug234814 /none/tests/bug491394 +/none/tests/bug492678 /none/tests/closeall /none/tests/coolo_sigaction /none/tests/coolo_strlen @@ -1645,6 +1646,7 @@ /none/tests/thread-exits /none/tests/threaded-fork /none/tests/threadederrno +/none/tests/timer_delete /none/tests/timestamp /none/tests/tls /none/tests/track-fds-exec-children diff --git a/coregrind/m_syswrap/syswrap-freebsd.c b/coregrind/m_syswrap/syswrap-freebsd.c index d358c90eb0..3397249383 100644 --- a/coregrind/m_syswrap/syswrap-freebsd.c +++ b/coregrind/m_syswrap/syswrap-freebsd.c @@ -2411,6 +2411,7 @@ PRE(sys_timer_delete) { PRINT("sys_timer_delete( %#" FMT_REGWORD "x )", ARG1); PRE_REG_READ1(long, "timer_delete", vki_timer_t, timerid); + *flags |= SfPollAfter; } // SYS_ktimer_settime 237 diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c index a74b8f1d21..c281021a9f 100644 --- a/coregrind/m_syswrap/syswrap-generic.c +++ b/coregrind/m_syswrap/syswrap-generic.c @@ -2959,6 +2959,14 @@ PRE(sys_setitimer) &(value->it_interval)); PRE_timeval_READ( "setitimer(&value->it_value)", &(value->it_value)); + // "Setting it_value to 0 disables a timer" + // poll for signals in that case + if (ML_(safe_to_deref)(value, sizeof(*value))) { + if (value->it_value.tv_sec == 0 && + value->it_value.tv_usec == 0) { + *flags |= SfPollAfter; + } + } } if (ARG3 != (Addr)NULL) { struct vki_itimerval *ovalue = (struct vki_itimerval*)(Addr)ARG3; diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c index e8978b5bc1..83af91344e 100644 --- a/coregrind/m_syswrap/syswrap-linux.c +++ b/coregrind/m_syswrap/syswrap-linux.c @@ -3244,6 +3244,7 @@ PRE(sys_timer_delete) { PRINT("sys_timer_delete( %#" FMT_REGWORD "x )", ARG1); PRE_REG_READ1(long, "timer_delete", vki_timer_t, timerid); + *flags |= SfPollAfter; } /* --------------------------------------------------------------------- diff --git a/coregrind/m_syswrap/syswrap-solaris.c b/coregrind/m_syswrap/syswrap-solaris.c index 104a809d3d..9abeb85001 100644 --- a/coregrind/m_syswrap/syswrap-solaris.c +++ b/coregrind/m_syswrap/syswrap-solaris.c @@ -8531,6 +8531,7 @@ PRE(sys_timer_delete) /* int timer_delete(timer_t timerid); */ PRINT("sys_timer_delete ( %ld )", SARG1); PRE_REG_READ1(long, "timer_delete", vki_timer_t, timerid); + *flags |= SfPollAfter; } PRE(sys_timer_settime) diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index 3dc8762514..2fa43587e2 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -103,6 +103,7 @@ EXTRA_DIST = \ bug129866.vgtest bug129866.stderr.exp bug129866.stdout.exp \ bug234814.vgtest bug234814.stderr.exp bug234814.stdout.exp \ bug491394.vgtest bug491394.stderr.exp \ + bug492678.vgtest bug492678.stderr.exp \ closeall.stderr.exp closeall.vgtest \ cmdline0.stderr.exp cmdline0.stdout.exp cmdline0.vgtest \ cmdline1.stderr.exp cmdline1.stdout.exp cmdline1.vgtest \ @@ -241,6 +242,7 @@ EXTRA_DIST = \ threaded-fork.stderr.exp threaded-fork.stdout.exp threaded-fork.vgtest \ threadederrno.stderr.exp threadederrno.stdout.exp \ threadederrno.vgtest \ + timer_delete.vgtest timer_delete.stderr.exp \ timestamp.stderr.exp timestamp.vgtest \ tls.vgtest tls.stderr.exp tls.stdout.exp \ track-fds-exec-children.vgtest track-fds-exec-children.stderr.exp \ @@ -267,7 +269,7 @@ check_PROGRAMS = \ args \ async-sigs \ bitfield1 \ - bug129866 bug234814 \ + bug129866 bug234814 bug492678\ closeall coolo_strlen \ discard exec-sigmask execve faultstatus fcntl_setown \ fdleak_cmsg fdleak_creat fdleak_dup fdleak_dup2 \ @@ -300,6 +302,7 @@ check_PROGRAMS = \ thread-exits \ threaded-fork \ threadederrno \ + timer_delete \ timestamp \ tls \ tls.so \ @@ -439,6 +442,7 @@ thread_exits_LDADD = -lpthread threaded_fork_LDADD = -lpthread threadederrno_CFLAGS = $(AM_CFLAGS) threadederrno_LDADD = -lpthread +timer_delete_LDADD = -lrt timestamp_CFLAGS = ${AM_CFLAGS} @FLAG_W_NO_USE_AFTER_FREE@ tls_SOURCES = tls.c tls2.c tls_DEPENDENCIES = tls.so tls2.so diff --git a/none/tests/bug492678.c b/none/tests/bug492678.c new file mode 100644 index 0000000000..6e4ca80f4d --- /dev/null +++ b/none/tests/bug492678.c @@ -0,0 +1,104 @@ +#include <errno.h> +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/time.h> + +#define CHECK_RES(res) \ + if (res == -1) \ + { \ + fprintf(stderr, "unexpected error at line %d: %s", __LINE__, strerror(errno)); \ + exit(EXIT_FAILURE); \ + } + +static int tries = 0; + +static void sigalrm_handler(int signum) +{ + (void)signum; + // no need to do anything for the experiment +} + +// using this handler in place of SIG_DFL to monitor when the "default" handler is called +static void sigalrm_default_handler(int signum) +{ + (void)signum; + fprintf(stderr, + "ERROR: unreachable code reached: sigalarm timer was disarmed, but still received SIGALRM (race condition " + "tried %d times)\n", + tries); + exit(EXIT_FAILURE); +} + +void sigalrm_timer_destroy(void) +{ + int res; + + // REMOVE TIMER + const struct itimerval zero = { + .it_interval = + { + .tv_sec = 0, + .tv_usec = 0, + }, + .it_value = + { + .tv_sec = 0, + .tv_usec = 0, + }, + }; + res = setitimer(ITIMER_REAL, &zero, NULL); + CHECK_RES(res) + + // AND THEN REMOVE SIGNAL HANDLER + struct sigaction sigact; + sigact.sa_flags = 0; + sigact.sa_handler = sigalrm_default_handler; // using this handler in place of SIG_DFL to monitor when the "default" + // handler is called + res = sigemptyset(&sigact.sa_mask); + CHECK_RES(res) + res = sigaction(SIGALRM, &sigact, NULL); + CHECK_RES(res) +} + +void sigalrm_timer_setup(const struct timeval *period) +{ + int res; + + // SIGNAL + struct sigaction sigact; + sigact.sa_flags = 0; + sigact.sa_handler = sigalrm_handler; + res = sigemptyset(&sigact.sa_mask); + CHECK_RES(res) + res = sigaction(SIGALRM, &sigact, NULL); + CHECK_RES(res) + + // TIMER + const struct itimerval timer_val = { + .it_interval = *period, + .it_value = *period, + }; + res = setitimer(ITIMER_REAL, &timer_val, NULL); + CHECK_RES(res); +} + +static void try_race_condition(void) +{ + ++tries; + const struct timeval clk_period = { + .tv_sec = 0, + .tv_usec = 1, + }; + sigalrm_timer_setup(&clk_period); + sigalrm_timer_destroy(); +} + +int main(void) +{ + for (int i = 0; i < 10000; ++i) + { + try_race_condition(); + } +} diff --git a/none/tests/bug492678.stderr.exp b/none/tests/bug492678.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/bug492678.vgtest b/none/tests/bug492678.vgtest new file mode 100644 index 0000000000..39ff27650a --- /dev/null +++ b/none/tests/bug492678.vgtest @@ -0,0 +1,3 @@ +prog: bug492678 +vgopts: -q + diff --git a/none/tests/timer_delete.c b/none/tests/timer_delete.c new file mode 100644 index 0000000000..4a103d8f88 --- /dev/null +++ b/none/tests/timer_delete.c @@ -0,0 +1,102 @@ +#include <errno.h> +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <time.h> + +timer_t timerid; + +#define CHECK_RES(res) \ + if (res == -1) \ + { \ + fprintf(stderr, "unexpected error at line %d: %s", __LINE__, strerror(errno)); \ + exit(EXIT_FAILURE); \ + } + +static int tries = 0; + +static void sigalrm_handler(int signum) +{ + (void)signum; + // no need to do anything for the experiment +} + +// using this handler in place of SIG_DFL to monitor when the "default" handler is called +static void sigalrm_default_handler(int signum) +{ + (void)signum; + fprintf(stderr, + "ERROR: unreachable code reached: sigalarm timer was disarmed, but still received SIGALRM (race condition " + "tried %d times)\n", + tries); + exit(EXIT_FAILURE); +} + +void sigalrm_timer_destroy(void) +{ + int res; + + // REMOVE TIMER + res = timer_delete(timerid); + CHECK_RES(res) + + // AND THEN REMOVE SIGNAL HANDLER + struct sigaction sigact; + sigact.sa_flags = 0; + sigact.sa_handler = sigalrm_default_handler; // using this handler in place of SIG_DFL to monitor when the "default" + // handler is called + res = sigemptyset(&sigact.sa_mask); + CHECK_RES(res) + res = sigaction(SIGALRM, &sigact, NULL); + CHECK_RES(res) +} + +void sigalrm_timer_setup(const struct timespec *period) +{ + int res; + + // SIGNAL + struct sigaction sigact; + sigact.sa_flags = 0; + sigact.sa_handler = sigalrm_handler; + res = sigemptyset(&sigact.sa_mask); + CHECK_RES(res) + res = sigaction(SIGALRM, &sigact, NULL); + CHECK_RES(res) + + // SIGEVENT + struct sigevent sev; + sev.sigev_notify = SIGEV_SIGNAL; + sev.sigev_signo = SIGALRM; + sev.sigev_value.sival_ptr = &timerid; + res = timer_create(CLOCK_REALTIME, &sev, &timerid); + CHECK_RES(res); + + // TIMER + const struct itimerspec timer_val = { + .it_interval = *period, + .it_value = *period, + }; + res = timer_settime(timerid, 0, &timer_val, NULL); + CHECK_RES(res); +} + +static void try_race_condition(void) +{ + ++tries; + const struct timespec clk_period = { + .tv_sec = 0, + .tv_nsec = 20000 + }; + sigalrm_timer_setup(&clk_period); + sigalrm_timer_destroy(); +} + +int main(void) +{ + for (int i = 0; i < 10000; ++i) + { + try_race_condition(); + } +} diff --git a/none/tests/timer_delete.stderr.exp b/none/tests/timer_delete.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 |
