From 685dbc44107298f725178e1e648d9959a1cf2a4f Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 21 Feb 2025 16:26:24 +0100 Subject: [PATCH] MINOR: tinfo: add a new thread flag to indicate a call from a sig handler Signal handlers must absolutely not change anything, but some long and complex call chains may look innocuous at first glance, yet result in some subtle write accesses (e.g. pools) that can conflict with a running thread being interrupted. Let's add a new thread flag TH_FL_IN_SIG_HANDLER that is only set when entering a signal handler and cleared when leaving them. Note, we're speaking about real signal handlers (synchronous ones), not deferred ones. This will allow some sensitive call places to act differently when detecting such a condition, and possibly even to place a few new BUG_ON(). (cherry picked from commit ddd173355c9c7452ff6ec317c8be6195d25dba2a) Signed-off-by: Christopher Faulet (cherry picked from commit bc35f7763b413370f611c775e64acab469dead04) Signed-off-by: Amaury Denoyelle --- include/haproxy/tinfo-t.h | 1 + src/debug.c | 5 +++++ src/signal.c | 5 +++++ src/wdt.c | 8 ++++++++ 4 files changed, 19 insertions(+) diff --git a/include/haproxy/tinfo-t.h b/include/haproxy/tinfo-t.h index aa73179..0d17f40 100644 --- a/include/haproxy/tinfo-t.h +++ b/include/haproxy/tinfo-t.h @@ -65,6 +65,7 @@ enum { #define TH_FL_STARTED 0x00000010 /* set once the thread starts */ #define TH_FL_IN_LOOP 0x00000020 /* set only inside the polling loop */ #define TH_FL_DUMPING_OTHERS 0x00000040 /* thread currently dumping other threads */ +#define TH_FL_IN_SIG_HANDLER 0x00000080 /* thread currently in signal handler */ /* we have 4 buffer-wait queues, in highest to lowest emergency order */ #define DYNBUF_NBQ 4 diff --git a/src/debug.c b/src/debug.c index b472e01..cf3c50a 100644 --- a/src/debug.c +++ b/src/debug.c @@ -2296,6 +2296,9 @@ void debug_handler(int sig, siginfo_t *si, void *arg) if (!buf || (ulong)buf & 0x1UL) return; + /* inform callees to be careful, we're in a signal handler! */ + _HA_ATOMIC_OR(&th_ctx->flags, TH_FL_IN_SIG_HANDLER); + /* Special value 0x2 is used during panics and requires that the thread * allocates its own dump buffer among its own trash buffers. The goal * is that all threads keep a copy of their own dump. @@ -2316,6 +2319,8 @@ void debug_handler(int sig, siginfo_t *si, void *arg) */ while (no_return) wait(NULL); + + _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER); } static int init_debug_per_thread() diff --git a/src/signal.c b/src/signal.c index 9dfb5dc..e5ca614 100644 --- a/src/signal.c +++ b/src/signal.c @@ -38,10 +38,14 @@ DECLARE_STATIC_POOL(pool_head_sig_handlers, "sig_handlers", sizeof(struct sig_ha */ void signal_handler(int sig) { + /* inform callees to be careful, we're in a signal handler! */ + _HA_ATOMIC_OR(&th_ctx->flags, TH_FL_IN_SIG_HANDLER); + if (sig < 0 || sig >= MAX_SIGNAL) { /* unhandled signal */ signal(sig, SIG_IGN); qfprintf(stderr, "Received unhandled signal %d. Signal has been disabled.\n", sig); + _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER); return; } @@ -59,6 +63,7 @@ void signal_handler(int sig) /* If the thread is TH_FL_SLEEPING we need to wake it */ wake_thread(tid); + _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER); } /* Call handlers of all pending signals and clear counts and queue length. The diff --git a/src/wdt.c b/src/wdt.c index 9b5d2d6..fbf0a63 100644 --- a/src/wdt.c +++ b/src/wdt.c @@ -68,6 +68,9 @@ void wdt_handler(int sig, siginfo_t *si, void *arg) ulong thr_bit; int thr, tgrp; + /* inform callees to be careful, we're in a signal handler! */ + _HA_ATOMIC_OR(&th_ctx->flags, TH_FL_IN_SIG_HANDLER); + switch (si->si_code) { case SI_TIMER: /* A thread's timer fired, the thread ID is in si_int. We have @@ -163,6 +166,7 @@ void wdt_handler(int sig, siginfo_t *si, void *arg) #endif default: /* unhandled other conditions */ + _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER); return; } @@ -177,10 +181,14 @@ void wdt_handler(int sig, siginfo_t *si, void *arg) else #endif ha_panic(); + + _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER); return; update_and_leave: wdt_ping(thr); + + _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER); } /* parse the "warn-blocked-traffic-after" parameter */ -- 1.7.10.4