summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDenys Fedoryshchenko <denys.f@collabora.com>2026-07-08 01:57:08 +0300
committerDenys Fedoryshchenko <denys.f@collabora.com>2026-07-08 02:53:26 +0300
commit844812959beb1a6bbcc64099a4f85b3db127a8a4 (patch)
tree0c494e3b141ac5e53e1f18d05809b84be95f3ca3
parentf4014a4a2c9e654646faeb81cd9ac5841b1c9b0f (diff)
downloadaccel-ppp-844812959beb1a6bbcc64099a4f85b3db127a8a4.tar.gz
accel-ppp-844812959beb1a6bbcc64099a4f85b3db127a8a4.zip
triton: reject concurrent config reload requests
triton_conf_reload() kept the notify callback in a single global slot, and the CLI reload command likewise stored its wakeup context in a global. A second reload issued while the first was still pending (from another CLI connection or SIGUSR1) overwrote both, so only the last requester was notified when the reload completed; the earlier CLI context slept forever in triton_context_schedule() and that connection hung while the rest of the daemon kept running. Make triton_conf_reload() return -1 when a reload is already pending, checked atomically under threads_lock, and pass a caller-provided arg through to the notify callback so each requester keeps its own state instead of sharing globals. The CLI reload's request struct is heap-allocated rather than kept on reload_exec's stack, since triton_context_schedule() can migrate a suspended context onto a different worker thread's stack before the notify callback runs, which would otherwise leave conf_reload_notify() writing through a stale stack pointer. Also mark the reload as running (need_config_reload = 2) before dropping threads_lock to call __config_reload(). Previously a worker woken during the reload (e.g. by the notify callback waking the CLI context, or any stray context wakeup) could loop through the idle path, decrement the active count back to zero and re-enter __config_reload() while need_config_reload was still set, running a second concurrent conf_reload() and invoking the notify callback twice - corrupting the wakeup list and, with the CLI's heap request, writing through freed memory. The SIGUSR1 handler used to call triton_conf_reload() directly from signal context, taking spinlocks and potentially running the whole config parse inside the handler. It now only sets a flag; the main thread waits with sigtimedwait() and performs the reload (and logs a warning when one is already in progress) from normal thread context. The CLI now replies "reload is already in progress" instead of losing the first requester's wakeup.
-rw-r--r--accel-pppd/cli/std_cmd.c41
-rw-r--r--accel-pppd/main.c20
-rw-r--r--accel-pppd/triton/triton.c30
-rw-r--r--accel-pppd/triton/triton.h2
4 files changed, 67 insertions, 26 deletions
diff --git a/accel-pppd/cli/std_cmd.c b/accel-pppd/cli/std_cmd.c
index 951b10da..1d15a28c 100644
--- a/accel-pppd/cli/std_cmd.c
+++ b/accel-pppd/cli/std_cmd.c
@@ -340,26 +340,41 @@ static int shutdown_exec(const char *cmd, char * const *f, int f_cnt, void *cli)
}
//==========================
-static int conf_reload_res;
-static struct triton_context_t *conf_reload_ctx;
-static void conf_reload_notify(int r)
+struct conf_reload_req {
+ struct triton_context_t *ctx;
+ int res;
+};
+static void conf_reload_notify(int r, void *arg)
{
+ struct conf_reload_req *req = arg;
+
if (!r)
triton_event_fire(EV_CONFIG_RELOAD, NULL);
- conf_reload_res = r;
- triton_context_wakeup(conf_reload_ctx);
+ req->res = r;
+ triton_context_wakeup(req->ctx);
}
static int reload_exec(const char *cmd, char * const *f, int f_cnt, void *cli)
{
- if (f_cnt == 1) {
- conf_reload_ctx = triton_context_self();
- triton_conf_reload(conf_reload_notify);
- triton_context_schedule();
- if (conf_reload_res)
- cli_send(cli, "failed\r\n");
- return CLI_CMD_OK;
- } else
+ struct conf_reload_req *req;
+
+ if (f_cnt != 1)
return CLI_CMD_SYNTAX;
+
+ /* heap-allocated: triton_context_schedule() can migrate this
+ * context to another worker thread's stack before notify runs */
+ req = _malloc(sizeof(*req));
+ req->ctx = triton_context_self();
+
+ if (triton_conf_reload(conf_reload_notify, req)) {
+ _free(req);
+ cli_send(cli, "reload is already in progress\r\n");
+ return CLI_CMD_OK;
+ }
+ triton_context_schedule();
+ if (req->res)
+ cli_send(cli, "failed\r\n");
+ _free(req);
+ return CLI_CMD_OK;
}
static void reload_help(char * const *fields, int fields_cnt, void *client)
diff --git a/accel-pppd/main.c b/accel-pppd/main.c
index 515b8036..221be7be 100644
--- a/accel-pppd/main.c
+++ b/accel-pppd/main.c
@@ -41,6 +41,8 @@ static int term;
static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+static volatile sig_atomic_t need_reload;
+
#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
static pthread_mutex_t *ssl_lock_cs;
@@ -100,7 +102,7 @@ static void change_limits(void)
log_emerg("main: setrlimit: %s\n", strerror(errno));
}
-static void config_reload_notify(int r)
+static void config_reload_notify(int r, void *arg)
{
if (!r)
triton_event_fire(EV_CONFIG_RELOAD, NULL);
@@ -108,7 +110,7 @@ static void config_reload_notify(int r)
static void config_reload(int num)
{
- triton_conf_reload(config_reload_notify);
+ need_reload = 1;
}
static void close_all_fd(void)
@@ -437,7 +439,19 @@ int main(int _argc, char **_argv)
backup_restore(internal);
#endif
- sigwait(&set, &sig);
+ {
+ struct timespec ts = { .tv_sec = 1 };
+ while (1) {
+ sig = sigtimedwait(&set, NULL, &ts);
+ if (sig > 0)
+ break;
+ if (need_reload) {
+ need_reload = 0;
+ if (triton_conf_reload(config_reload_notify, NULL))
+ log_warn("main: config reload is already in progress\n");
+ }
+ }
+ }
log_info1("terminate, sig = %i\n", sig);
ap_shutdown_soft(shutdown_cb, 1);
diff --git a/accel-pppd/triton/triton.c b/accel-pppd/triton/triton.c
index 5c7231b7..105f810a 100644
--- a/accel-pppd/triton/triton.c
+++ b/accel-pppd/triton/triton.c
@@ -32,8 +32,10 @@ static LIST_HEAD(init_list);
static int terminate;
static int need_terminate;
+/* 0 - idle, 1 - reload requested, 2 - reload running */
static int need_config_reload;
-static void (*config_reload_notify)(int);
+static void (*config_reload_notify)(int, void *);
+static void *config_reload_arg;
static mempool_t *ctx_pool;
static mempool_t *call_pool;
@@ -210,14 +212,14 @@ void triton_thread_wakeup(struct _triton_thread_t *thread)
pthread_kill(thread->thread, SIGUSR1);
}
-static void __config_reload(void (*notify)(int))
+static void __config_reload(void)
{
struct _triton_thread_t *t;
int r;
log_debug2("config_reload: enter\n");
r = conf_reload(NULL);
- notify(r);
+ config_reload_notify(r, config_reload_arg);
spin_lock(&threads_lock);
need_config_reload = 0;
@@ -301,9 +303,10 @@ static void* triton_thread(struct _triton_thread_t *thread)
if (!terminate)
list_add(&thread->entry2, &sleep_threads);
- if (triton_stat_thread_active_dec() == 0 && need_config_reload) {
+ if (triton_stat_thread_active_dec() == 0 && need_config_reload == 1) {
+ need_config_reload = 2;
spin_unlock(&threads_lock);
- __config_reload(config_reload_notify);
+ __config_reload();
} else
spin_unlock(&threads_lock);
@@ -856,16 +859,25 @@ int __export triton_load_modules(const char *mod_sect)
return 0;
}
-void __export triton_conf_reload(void (*notify)(int))
+int __export triton_conf_reload(void (*notify)(int, void *), void *arg)
{
spin_lock(&threads_lock);
- need_config_reload = 1;
+ if (need_config_reload) {
+ spin_unlock(&threads_lock);
+ return -1;
+ }
config_reload_notify = notify;
+ config_reload_arg = arg;
if (triton_stat_thread_active() == 0) {
+ need_config_reload = 2;
spin_unlock(&threads_lock);
- __config_reload(notify);
- } else
+ __config_reload();
+ } else {
+ need_config_reload = 1;
spin_unlock(&threads_lock);
+ }
+
+ return 0;
}
void __export triton_run()
diff --git a/accel-pppd/triton/triton.h b/accel-pppd/triton/triton.h
index aef77c2b..63dc9188 100644
--- a/accel-pppd/triton/triton.h
+++ b/accel-pppd/triton/triton.h
@@ -104,7 +104,7 @@ void triton_event_fire(int ev_id, void *arg);
struct conf_sect_t *conf_get_section(const char *name);
char *conf_get_opt(const char *sect, const char *name);
-void triton_conf_reload(void (*notify)(int));
+int triton_conf_reload(void (*notify)(int, void *), void *arg);
void triton_collect_cpu_usage(void);
void triton_stop_collect_cpu_usage(void);