From b339cc149751429c352d8537f2d0c8ecb9a6f80f Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Mon, 21 Sep 2020 15:57:59 -0400 Subject: [PATCH] lib: avoid signal-handling race with event loop poll call Manage the main pthread's signal mask to avoid a signal-handling race. Before entering poll, check for pending signals that the application needs to handle. Use ppoll() to re-enable those signals during the poll call. Signed-off-by: Mark Stapp --- lib/thread.c | 73 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 13 deletions(-) diff --git a/lib/thread.c b/lib/thread.c index f83cbea68a..8d840359e4 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -723,9 +723,13 @@ static void thread_free(struct thread_master *master, struct thread *thread) XFREE(MTYPE_THREAD, thread); } -static int fd_poll(struct thread_master *m, struct pollfd *pfds, nfds_t pfdsize, - nfds_t count, const struct timeval *timer_wait) +static int fd_poll(struct thread_master *m, const struct timeval *timer_wait, + bool *eintr_p) { + sigset_t origsigs; + unsigned char trash[64]; + nfds_t count = m->handler.copycount; + /* * If timer_wait is null here, that means poll() should block * indefinitely, unless the thread_master has overridden it by setting @@ -756,15 +760,58 @@ static int fd_poll(struct thread_master *m, struct pollfd *pfds, nfds_t pfdsize, rcu_assert_read_unlocked(); /* add poll pipe poker */ - assert(count + 1 < pfdsize); - pfds[count].fd = m->io_pipe[0]; - pfds[count].events = POLLIN; - pfds[count].revents = 0x00; + assert(count + 1 < m->handler.pfdsize); + m->handler.copy[count].fd = m->io_pipe[0]; + m->handler.copy[count].events = POLLIN; + m->handler.copy[count].revents = 0x00; + + /* We need to deal with a signal-handling race here: we + * don't want to miss a crucial signal, such as SIGTERM or SIGINT, + * that may arrive just before we enter poll(). We will block the + * key signals, then check whether any have arrived - if so, we return + * before calling poll(). If not, we'll re-enable the signals + * in the ppoll() call. + */ + + sigemptyset(&origsigs); + if (m->handle_signals) { + /* Main pthread that handles the app signals */ + if (frr_sigevent_check(&origsigs)) { + /* Signal to process - restore signal mask and return */ + pthread_sigmask(SIG_SETMASK, &origsigs, NULL); + num = -1; + *eintr_p = true; + goto done; + } + } else { + /* Don't make any changes for the non-main pthreads */ + pthread_sigmask(SIG_SETMASK, NULL, &origsigs); + } - num = poll(pfds, count + 1, timeout); +#if defined(HAVE_PPOLL) + struct timespec ts, *tsp; - unsigned char trash[64]; - if (num > 0 && pfds[count].revents != 0 && num--) + if (timeout >= 0) { + ts.tv_sec = timeout / 1000; + ts.tv_nsec = (timeout % 1000) * 1000000; + tsp = &ts; + } else + tsp = NULL; + + num = ppoll(m->handler.copy, count + 1, tsp, &origsigs); + pthread_sigmask(SIG_SETMASK, &origsigs, NULL); +#else + /* Not ideal - there is a race after we restore the signal mask */ + pthread_sigmask(SIG_SETMASK, &origsigs, NULL); + num = poll(m->handler.copy, count + 1, timeout); +#endif + +done: + + if (num < 0 && errno == EINTR) + *eintr_p = true; + + if (num > 0 && m->handler.copy[count].revents != 0 && num--) while (read(m->io_pipe[0], &trash, sizeof(trash)) > 0) ; @@ -1391,7 +1438,7 @@ struct thread *thread_fetch(struct thread_master *m, struct thread *fetch) struct timeval zerotime = {0, 0}; struct timeval tv; struct timeval *tw = NULL; - + bool eintr_p = false; int num = 0; do { @@ -1463,14 +1510,14 @@ struct thread *thread_fetch(struct thread_master *m, struct thread *fetch) pthread_mutex_unlock(&m->mtx); { - num = fd_poll(m, m->handler.copy, m->handler.pfdsize, - m->handler.copycount, tw); + eintr_p = false; + num = fd_poll(m, tw, &eintr_p); } pthread_mutex_lock(&m->mtx); /* Handle any errors received in poll() */ if (num < 0) { - if (errno == EINTR) { + if (eintr_p) { pthread_mutex_unlock(&m->mtx); /* loop around to signal handler */ continue; -- 2.39.5