From: Roy Marples Date: Mon, 18 Feb 2013 20:56:55 +0000 (+0000) Subject: Rework our signal setup and event loop around ppoll(2). X-Git-Tag: v5.99.6~59 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e6d25175623a7ec71a7a2ea0cc0bdc192acd463f;p=thirdparty%2Fdhcpcd.git Rework our signal setup and event loop around ppoll(2). ppoll is mapped to pollts(2) for supporting systems. If pselect(2) is available then we provide a compat shim for that, otherwise we provide a non race free shim based on poll(2). --- diff --git a/compat/ppoll.c b/compat/ppoll.c new file mode 100644 index 00000000..fa338ba1 --- /dev/null +++ b/compat/ppoll.c @@ -0,0 +1,63 @@ +/* + * dhcpcd - DHCP client daemon + * Copyright (c) 2006-2013 Roy Marples + * All rights reserved + + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include +#include + +#include +#include +#include +#include + +#include "ppoll.h" + +#warning "This ppoll(2) implementation is not entirely race condition safe." +#warning "Only operating system support for ppoll(2) can correct this." + +int +ppoll(struct pollfd *restrict fds, nfds_t nfds, + const struct timespec *restrict ts, const sigset_t *restrict sigmask) +{ + int r, timeout; + sigset_t oldset; + + if (ts == NULL) + timeout = -1; + else if (ts->tv_sec > INT_MAX / 1000 || + (ts->tv_sec == INT_MAX / 1000 && + (ts->tv_nsec + 999999) / 1000000 > INT_MAX % 1000000)) + timeout = INT_MAX; + else + timeout = ts->tv_sec * 1000 + (ts->tv_nsec + 999999) / 1000000; + if (sigmask && sigprocmask(SIG_SETMASK, sigmask, &oldset) == -1) + return -1; + r = poll(fds, nfds, timeout); + if (sigmask && sigprocmask(SIG_SETMASK, &oldset, NULL) == -1) + return -1; + + return r; +} diff --git a/compat/pselect.h b/compat/ppoll.h similarity index 83% rename from compat/pselect.h rename to compat/ppoll.h index 2d61ce36..45911a71 100644 --- a/compat/pselect.h +++ b/compat/ppoll.h @@ -1,6 +1,6 @@ /* * dhcpcd - DHCP client daemon - * Copyright (c) 2006-2012 Roy Marples + * Copyright (c) 2006-2013 Roy Marples * All rights reserved * Redistribution and use in source and binary forms, with or without @@ -25,16 +25,15 @@ * SUCH DAMAGE. */ -#ifndef PSELECT_H -#define PSELECT_H - -#include -#include +#ifndef PPOLL_H +#define PPOLL_H +#include #include +#include -int pselect(int, fd_set *restrict, fd_set *restrict, fd_set *restrict, - const struct timespec *restrict, - const sigset_t *restrict); +int +ppoll(struct pollfd *restrict, nfds_t, const struct timespec *restrict, + const sigset_t *restrict); #endif diff --git a/compat/pselect.c b/compat/pselect.c index 79302158..5c5197ed 100644 --- a/compat/pselect.c +++ b/compat/pselect.c @@ -1,6 +1,6 @@ /* * dhcpcd - DHCP client daemon - * Copyright (c) 2006-2012 Roy Marples + * Copyright (c) 2006-2013 Roy Marples * All rights reserved * Redistribution and use in source and binary forms, with or without @@ -28,38 +28,38 @@ #include #include +#include +#include #include #include -#include "pselect.h" - -#warning "This pselect(2) implementation is not entirely race condition safe." -#warning "Only operating system support for pselect(2) can correct this." +#include "ppoll.h" int -pselect(int nfds, - fd_set *restrict readfds, - fd_set *restrict writefds, - fd_set *restrict errorfds, - const struct timespec *restrict timeout, - const sigset_t *restrict newset) +ppoll(struct pollfd *restrict fds, nfds_t nfds, + const struct timespec *restrict ts, const sigset_t *restrict sigmask) { - int r; - sigset_t oldset; - struct timeval saved_timeout; - - if (newset && sigprocmask(SIG_SETMASK, newset, &oldset) == -1) - return -1; + fd_set read_fds; + nfds_t n; + int maxfd, r; - if (timeout) { - saved_timeout.tv_sec = timeout->tv_sec; - saved_timeout.tv_usec = timeout->tv_nsec / 1000; - r = select(nfds, readfds, writefds, errorfds, &saved_timeout); - } else - r = select(nfds, readfds, writefds, errorfds, NULL); + FD_ZERO(&read_fds); + maxfd = 0; + for (n = 0; n < nfds; n++) { + if (fds[n].events & POLLIN) { + FD_SET(fds[n].fd, &read_fds); + if (fds[n].fd > maxfd) + maxfd = fds[n].fd; + } + } - if (newset && sigprocmask(SIG_SETMASK, &oldset, NULL) == -1) - return -1; + r = pselect(maxfd + 1, &read_fds, NULL, NULL, ts, sigmask); + if (r > 0) { + for (n = 0; n < nfds; n++) { + fds[n].revents = + FD_ISSET(fds[n].fd, &read_fds) ? POLLIN : 0; + } + } return r; } diff --git a/configure b/configure index 7d684414..b2b5ddce 100755 --- a/configure +++ b/configure @@ -57,7 +57,7 @@ for x do --without-getline) GETLINE=no;; --without-strlcpy) STRLCPY=no;; --without-posix_spawn) POSIX_SPAWN=no;; - --without-pselect) PSELECT=no;; + --without-ppoll) PPOLL=no;; --serviceexists) SERVICEEXISTS=$var;; --servicecmd) SERVICECMD=$var;; --servicestatus) SERVICESTATUS=$var;; @@ -283,7 +283,7 @@ if [ -n "$DEBUG" -a "$DEBUG" != no -a "$DEBUG" != false ]; then elif [ -z "$DEBUG" -a -d .git ]; then printf "Found git ... " DEBUG=yes - echo "CFLAGS+= -ggdb" >>$CONFIG_MK + echo "CFLAGS+= -ggdb" >>$CONFIG_MK else DEBUG=no fi @@ -525,7 +525,46 @@ else echo "#include " >>$CONFIG_H fi -if [ -z "$PSELECT" ]; then +if [ -z "$PPOLL" ]; then + printf "Testing for ppoll ... " + cat <_ppoll.c +#include +#include +int main(void) { + ppoll(NULL, 0, NULL, NULL); + return 0; +} +EOF + if $XCC _ppoll.c -o _ppoll 2>/dev/null; then + PPOLL=yes + else + PPOLL=no + fi + echo "$PPOLL" + rm -f _ppoll.c _ppoll +fi +if [ "$PPOLL" = no ]; then + printf "Testing for pollts ... " + cat <_pollts.c +#include +#include +#include +#include +int main(void) { + pollts(NULL, 0, NULL, NULL); + return 0; +} +EOF + if $XCC _pollts.c -o _pollts 2>/dev/null; then + PPOLL=pollts + echo "yes" + else + PPOLL=no + echo "no" + fi + rm -f _pollts.c _pollts +fi +if [ "$PPOLL" = no ]; then printf "Testing for pselect ... " cat <_pselect.c #include @@ -536,17 +575,27 @@ int main(void) { } EOF if $XCC _pselect.c -o _pselect 2>/dev/null; then - PSELECT=yes + PPOLL=pselect + echo "yes" else - PSELECT=no + PPOLL=no + echo "no" fi - echo "$PSELECT" rm -f _pselect.c _pselect fi -if [ "$PSELECT" = no ]; then +case "$PPOLL" in +pollts) + echo "#define ppoll pollts" >>$CONFIG_H + ;; +pselect) echo "COMPAT_SRCS+= compat/pselect.c" >>$CONFIG_MK - echo "#include \"compat/pselect.h\"" >>$CONFIG_H -fi + echo "#include \"compat/ppoll.h\"" >>$CONFIG_H + ;; +*) + echo "COMPAT_SRCS+= compat/ppoll.c" >>$CONFIG_MK + echo "#include \"compat/ppoll.h\"" >>$CONFIG_H + ;; +esac if [ -z "$SERVICECMD" ]; then printf "Checking for OpenRC ... " diff --git a/dhcpcd.c b/dhcpcd.c index 3fc05cc9..a305803d 100644 --- a/dhcpcd.c +++ b/dhcpcd.c @@ -1125,11 +1125,7 @@ main(int argc, char **argv) eloop_init(); #endif - /* This blocks all signals we're interested in. - * eloop uses pselect(2) so that the signals are unblocked - * when we're testing fd's. - * This allows us to ensure a consistent state is maintained - * regardless of when we are interrupted .*/ + /* Save signal mask, block and redirect signals to our handler */ if (signal_init(handle_signal, &dhcpcd_sigset) == -1) { syslog(LOG_ERR, "signal_setup: %m"); exit(EXIT_FAILURE); diff --git a/eloop.c b/eloop.c index 0f4191be..2a1d88c0 100644 --- a/eloop.c +++ b/eloop.c @@ -1,6 +1,6 @@ /* * dhcpcd - DHCP client daemon - * Copyright (c) 2006-2012 Roy Marples + * Copyright (c) 2006-2013 Roy Marples * All rights reserved * Redistribution and use in source and binary forms, with or without @@ -45,8 +45,10 @@ static struct event { int fd; void (*callback)(void *); void *arg; + struct pollfd *pollfd; struct event *next; } *events; +static size_t events_len; static struct event *free_events; static struct timeout { @@ -58,7 +60,24 @@ static struct timeout { } *timeouts; static struct timeout *free_timeouts; -void +static struct pollfd *fds; +static size_t fds_len; + +static void +eloop_event_setup_fds(void) +{ + struct event *e; + size_t i; + + for (e = events, i = 0; e; e = e->next, i++) { + fds[i].fd = e->fd; + fds[i].events = POLLIN; + fds[i].revents = 0; + e->pollfd = &fds[i]; + } +} + +int eloop_event_add(int fd, void (*callback)(void *), void *arg) { struct event *e, *last = NULL; @@ -68,7 +87,7 @@ eloop_event_add(int fd, void (*callback)(void *), void *arg) if (e->fd == fd) { e->callback = callback; e->arg = arg; - return; + return 0; } last = e; } @@ -81,10 +100,23 @@ eloop_event_add(int fd, void (*callback)(void *), void *arg) e = malloc(sizeof(*e)); if (e == NULL) { syslog(LOG_ERR, "%s: %m", __func__); - return; + return -1; + } + } + + /* Ensure we can actually listen to it */ + events_len++; + if (events_len > fds_len) { + fds_len += 5; + free(fds); + fds = malloc(sizeof(*fds) * fds_len); + if (fds == NULL) { + syslog(LOG_ERR, "%s: %m", __func__); + return -1; } } + /* Now populate the structure and add it to the list */ e->fd = fd; e->callback = callback; e->arg = arg; @@ -93,6 +125,9 @@ eloop_event_add(int fd, void (*callback)(void *), void *arg) last->next = e; else events = e; + + eloop_event_setup_fds(); + return 0; } void @@ -108,13 +143,15 @@ eloop_event_delete(int fd) events = e->next; e->next = free_events; free_events = e; + events_len--; + eloop_event_setup_fds(); break; } last = e; } } -void +int eloop_q_timeout_add_tv(int queue, const struct timeval *when, void (*callback)(void *), void *arg) { @@ -126,7 +163,7 @@ eloop_q_timeout_add_tv(int queue, /* Check for time_t overflow. */ if (timercmp(&w, &now, <)) { errno = ERANGE; - return; + return -1; } /* Remove existing timeout if present */ @@ -150,7 +187,7 @@ eloop_q_timeout_add_tv(int queue, t = malloc(sizeof(*t)); if (t == NULL) { syslog(LOG_ERR, "%s: %m", __func__); - return; + return -1; } } } @@ -168,19 +205,20 @@ eloop_q_timeout_add_tv(int queue, if (!timeouts || timercmp(&t->when, &timeouts->when, <)) { t->next = timeouts; timeouts = t; - return; + return 0; } for (tt = timeouts; tt->next; tt = tt->next) if (timercmp(&t->when, &tt->next->when, <)) { t->next = tt->next; tt->next = t; - return; + return 0; } tt->next = t; t->next = NULL; + return 0; } -void +int eloop_q_timeout_add_sec(int queue, time_t when, void (*callback)(void *), void *arg) { @@ -188,7 +226,7 @@ eloop_q_timeout_add_sec(int queue, time_t when, tv.tv_sec = when; tv.tv_usec = 0; - eloop_q_timeout_add_tv(queue, &tv, callback, arg); + return eloop_q_timeout_add_tv(queue, &tv, callback, arg); } /* This deletes all timeouts for the interface EXCEPT for ones with the @@ -297,15 +335,13 @@ eloop_init(void) #endif _noreturn void -eloop_start(const sigset_t *cursigs) +eloop_start(const sigset_t *sigmask) { - int n, max_fd; - fd_set read_fds; + int n; struct event *e; struct timeout *t; struct timeval tv; - struct timespec ts; - const struct timespec *tsp; + struct timespec ts, *tsp; for (;;) { /* Run all timeouts first */ @@ -326,30 +362,23 @@ eloop_start(const sigset_t *cursigs) /* No timeouts, so wait forever */ tsp = NULL; - max_fd = -1; - FD_ZERO(&read_fds); - for (e = events; e; e = e->next) { - FD_SET(e->fd, &read_fds); - if (e->fd > max_fd) - max_fd = e->fd; - } - if (tsp == NULL && max_fd == -1) { + if (tsp == NULL && events_len == 0) { syslog(LOG_ERR, "nothing to do"); exit(EXIT_FAILURE); } - n = pselect(max_fd + 1, &read_fds, NULL, NULL, tsp, cursigs); + n = ppoll(fds, events_len, tsp, sigmask); if (n == -1) { - if (errno == EINTR) + if (errno == EAGAIN || errno == EINTR) continue; - syslog(LOG_ERR, "pselect: %m"); + syslog(LOG_ERR, "poll: %m"); exit(EXIT_FAILURE); } /* Process any triggered events. */ if (n) { for (e = events; e; e = e->next) { - if (FD_ISSET(e->fd, &read_fds)) { + if (e->pollfd->revents & (POLLIN || POLLHUP)) { e->callback(e->arg); /* We need to break here as the * callback could destroy the next diff --git a/eloop.h b/eloop.h index 1d96f983..e0cb0e44 100644 --- a/eloop.h +++ b/eloop.h @@ -1,6 +1,6 @@ /* * dhcpcd - DHCP client daemon - * Copyright (c) 2006-2012 Roy Marples + * Copyright (c) 2006-2013 Roy Marples * All rights reserved * Redistribution and use in source and binary forms, with or without @@ -44,10 +44,10 @@ #define eloop_timeouts_delete(a, ...) \ eloop_q_timeouts_delete(ELOOP_QUEUE, a, __VA_ARGS__) -void eloop_event_add(int fd, void (*)(void *), void *); +int eloop_event_add(int fd, void (*)(void *), void *); void eloop_event_delete(int fd); -void eloop_q_timeout_add_sec(int queue, time_t, void (*)(void *), void *); -void eloop_q_timeout_add_tv(int queue, const struct timeval *, void (*)(void *), +int eloop_q_timeout_add_sec(int queue, time_t, void (*)(void *), void *); +int eloop_q_timeout_add_tv(int queue, const struct timeval *, void (*)(void *), void *); void eloop_q_timeout_delete(int, void (*)(void *), void *); void eloop_q_timeouts_delete(int, void *, void (*)(void *), ...); diff --git a/signals.c b/signals.c index fb101c06..58263c85 100644 --- a/signals.c +++ b/signals.c @@ -1,6 +1,6 @@ /* * dhcpcd - DHCP client daemon - * Copyright (c) 2006-2012 Roy Marples + * Copyright (c) 2006-2013 Roy Marples * All rights reserved * Redistribution and use in source and binary forms, with or without @@ -74,33 +74,40 @@ signal_read(_unused void *arg) } static int -signal_handle(void (*func)(int), sigset_t *oldset) +signal_handle(void (*func)(int)) { unsigned int i; struct sigaction sa; - sigset_t newset; memset(&sa, 0, sizeof(sa)); sa.sa_handler = func; sigemptyset(&sa.sa_mask); - if (oldset) - sigemptyset(&newset); - for (i = 0; handle_sigs[i]; i++) { if (sigaction(handle_sigs[i], &sa, NULL) == -1) return -1; - if (oldset) - sigaddset(&newset, handle_sigs[i]); } - if (oldset) - return sigprocmask(SIG_BLOCK, &newset, oldset); return 0; } +int +signal_setup(void) +{ + + return signal_handle(signal_handler); +} + +int +signal_reset(void) +{ + + return signal_handle(SIG_DFL); +} + int signal_init(void (*func)(int), sigset_t *oldset) { + sigset_t newset; if (pipe(signal_pipe) == -1) return -1; @@ -110,10 +117,14 @@ signal_init(void (*func)(int), sigset_t *oldset) set_cloexec(signal_pipe[1] == -1)) return -1; + sigfillset(&newset); + if (sigprocmask(SIG_SETMASK, &newset, oldset) == -1) + return -1; + /* Because functions we need to reboot/reconf out interfaces * are not async signal safe, we need to setup a signal pipe * so that the actual handler is executed in our event loop. */ signal_callback = func; eloop_event_add(signal_pipe[0], signal_read, NULL); - return signal_handle(signal_handler, oldset); + return signal_setup(); } diff --git a/signals.h b/signals.h index 9e674202..bb5a0d77 100644 --- a/signals.h +++ b/signals.h @@ -1,6 +1,6 @@ /* * dhcpcd - DHCP client daemon - * Copyright (c) 2006-2012 Roy Marples + * Copyright (c) 2006-2013 Roy Marples * All rights reserved * Redistribution and use in source and binary forms, with or without @@ -30,6 +30,8 @@ extern const int handle_sigs[]; +int signal_setup(void); +int signal_reset(void); int signal_init(void (*)(int), sigset_t *); #endif