Skip to content

Commit

Permalink
threads: safely disable signals on threads launched by main thr excep…
Browse files Browse the repository at this point in the history
…t srv startup
  • Loading branch information
libor-peltan-cznic authored and salzmdan committed Sep 27, 2024
1 parent 6b624a8 commit 9811419
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 23 deletions.
2 changes: 2 additions & 0 deletions Knot.files
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ src/contrib/spinlock.h
src/contrib/string.c
src/contrib/string.h
src/contrib/strtonum.h
src/contrib/threads.c
src/contrib/threads.h
src/contrib/time.c
src/contrib/time.h
src/contrib/toeplitz.h
Expand Down
2 changes: 2 additions & 0 deletions src/contrib/Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ libcontrib_la_SOURCES = \
contrib/string.c \
contrib/string.h \
contrib/strtonum.h \
contrib/threads.c \
contrib/threads.h \
contrib/time.c \
contrib/time.h \
contrib/toeplitz.h \
Expand Down
3 changes: 2 additions & 1 deletion src/contrib/conn_pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "contrib/conn_pool.h"

#include "contrib/sockaddr.h"
#include "contrib/threads.h"

conn_pool_t *global_conn_pool = NULL;
conn_pool_t *global_sessticket_pool = NULL;
Expand Down Expand Up @@ -111,7 +112,7 @@ conn_pool_t *conn_pool_init(size_t capacity, knot_timediff_t timeout,
free(pool);
return NULL;
}
if (pthread_create(&pool->closing_thread, NULL, closing_thread, pool) != 0) {
if (thread_create_nosignal(&pool->closing_thread, closing_thread, pool) != 0) {
pthread_mutex_destroy(&pool->mutex);
free(pool);
return NULL;
Expand Down
36 changes: 36 additions & 0 deletions src/contrib/threads.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/* Copyright (C) 2024 CZ.NIC, z.s.p.o. <[email protected]>
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

#include "threads.h"

#include <signal.h>

int thread_create_nosignal(pthread_t *thr, void *(*cb)(void*), void *ctx)
{
sigset_t tmp, orig;
sigfillset(&tmp);
sigdelset(&tmp, SIGBUS);
sigdelset(&tmp, SIGFPE);
sigdelset(&tmp, SIGILL);
sigdelset(&tmp, SIGSEGV);
pthread_sigmask(SIG_SETMASK, &tmp, &orig);

int ret = pthread_create(thr, NULL, cb, ctx);

pthread_sigmask(SIG_SETMASK, &orig, NULL);

return ret;
}
37 changes: 37 additions & 0 deletions src/contrib/threads.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* Copyright (C) 2024 CZ.NIC, z.s.p.o. <[email protected]>
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

#pragma once

#include <pthread.h>

/*!
* \brief New thread using pthread_create() but with signals blocked.
*
* \note Some signals that are unpractical to block are still allowed.
*
* \param thr Thread to be launched.
* \param cb Callback function to be launched in the thread.
* \param ctx Arbitrary context for the callback.
*
* \return Return value of ptherad_create().
*
* In order to avoid race conditions when the child thread is launched and
* the signals are blocked within that thread afterwards, this method
* temporarily blocks the signals in the parent thread, lets the child
* inherit them blocked, and re-enables in the parent afterwards.
*/
int thread_create_nosignal(pthread_t *thr, void *(*cb)(void *), void *ctx);
3 changes: 2 additions & 1 deletion src/knot/common/stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <urcu.h>

#include "contrib/files.h"
#include "contrib/threads.h"
#include "knot/common/stats.h"
#include "knot/common/log.h"
#include "knot/nameserver/query_module.h"
Expand Down Expand Up @@ -440,7 +441,7 @@ void stats_reconfigure(conf_t *conf, server_t *server)
return;
}

int ret = pthread_create(&stats.dumper, NULL, dumper, NULL);
int ret = thread_create_nosignal(&stats.dumper, dumper, NULL);
if (ret != 0) {
log_error("stats, failed to launch periodic dumping (%s)",
knot_strerror(knot_map_errno_code(ret)));
Expand Down
3 changes: 2 additions & 1 deletion src/knot/modules/cookies/cookies.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "knot/include/module.h"
#include "libknot/libknot.h"
#include "contrib/atomic.h"
#include "contrib/threads.h"
#include "contrib/string.h"
#include "libdnssec/random.h"

Expand Down Expand Up @@ -269,7 +270,7 @@ int cookies_load(knotd_mod_t *mod)
ctx->secret_lifetime = conf.single.integer;

// Start the secret rollover thread.
if (pthread_create(&ctx->update_secret, NULL, update_secret, (void *)mod)) {
if (thread_create_nosignal(&ctx->update_secret, update_secret, (void *)mod)) {
knotd_mod_log(mod, LOG_ERR, "failed to create the secret rollover thread");
free(ctx);
return KNOT_ERROR;
Expand Down
22 changes: 2 additions & 20 deletions src/utils/knotd/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "libdnssec/crypto.h"
#include "libknot/libknot.h"
#include "contrib/strtonum.h"
#include "contrib/threads.h"
#include "contrib/time.h"
#include "knot/ctl/process.h"
#include "knot/conf/conf.h"
Expand Down Expand Up @@ -65,7 +66,6 @@ typedef struct {
knot_ctl_t *ctl;
server_t *server;
pthread_t thread;
sigset_t sigmask;
int ret;
int thread_idx;
bool exclusive;
Expand Down Expand Up @@ -210,24 +210,6 @@ static void enable_signals(void)
pthread_sigmask(SIG_UNBLOCK, &mask, NULL);
}

/*! \brief Create a control thread with correct signals setting. */
static void create_thread_sigmask(pthread_t *thr, void *(*fcn)(void*), void *ctx,
sigset_t *out_mask)
{
/* Block all blockable signals. */
sigset_t mask;
sigfillset(&mask);
sigdelset(&mask, SIGBUS);
sigdelset(&mask, SIGFPE);
sigdelset(&mask, SIGILL);
sigdelset(&mask, SIGSEGV);
pthread_sigmask(SIG_SETMASK, &mask, out_mask);

pthread_create(thr, NULL, fcn, ctx);

pthread_sigmask(SIG_SETMASK, out_mask, NULL);
}

/*! \brief Drop POSIX 1003.1e capabilities. */
static void drop_capabilities(void)
{
Expand Down Expand Up @@ -326,7 +308,7 @@ static concurrent_ctl_ctx_t *find_free_ctx(concurrent_ctl_ctx_t *concurrent_ctxs
pthread_mutex_lock(&cctx->mutex);
switch (cctx->state) {
case CONCURRENT_EMPTY:
create_thread_sigmask(&cctx->thread, ctl_process_thread, cctx, &cctx->sigmask);
(void)thread_create_nosignal(&cctx->thread, ctl_process_thread, cctx);
break;
case CONCURRENT_IDLE:
knot_ctl_free(cctx->ctl);
Expand Down

0 comments on commit 9811419

Please sign in to comment.