diff --git a/common/event.c b/common/event.c index e32cc2974..32c162e5b 100644 --- a/common/event.c +++ b/common/event.c @@ -102,6 +102,10 @@ void sudo_ev_free(struct sudo_event *ev) { debug_decl(sudo_ev_free, SUDO_DEBUG_EVENT) + + /* Make sure ev is not in use before freeing it. */ + if (ev->base != NULL) + (void)sudo_ev_del(NULL, ev); free(ev); debug_return; } @@ -123,7 +127,7 @@ sudo_ev_add(struct sudo_event_base *base, struct sudo_event *ev, bool tohead) } } /* Clear pending delete so adding from callback works properly. */ - CLR(ev->events, SUDO_EV_DELETE); + CLR(ev->flags, SUDO_EV_DELETE); debug_return_int(0); } @@ -155,20 +159,30 @@ sudo_ev_del(struct sudo_event_base *base, struct sudo_event *ev) if (sudo_ev_del_impl(base, ev) != 0) debug_return_int(-1); - /* Unlink from list. */ + /* Unlink from event list. */ TAILQ_REMOVE(&base->events, ev, entries); - /* If we removed the pending event, replace it with the next one. */ - if (base->pending == ev) - base->pending = TAILQ_NEXT(ev, entries); + /* Unlink from active list and update base pointers as needed. */ + if (ISSET(ev->flags, SUDO_EV_ACTIVE)) { + TAILQ_REMOVE(&base->active, ev, active_entries); + if (ev == base->pending) + base->pending = TAILQ_NEXT(ev, active_entries); + if (ev == base->cur) + base->cur = NULL; + } /* Mark event unused. */ - ev->pfd_idx = -1; ev->base = NULL; + ev->flags = 0; + ev->pfd_idx = -1; debug_return_int(0); } +/* + * Run main event loop. + * Returns 0 on success, 1 if no events registered and -1 on error + */ int sudo_ev_loop(struct sudo_event_base *base, int flags) { @@ -184,8 +198,62 @@ sudo_ev_loop(struct sudo_event_base *base, int flags) flags |= SUDO_EVLOOP_ONCE; base->flags = 0; - /* Most work is done by the backend. */ - rc = sudo_ev_loop_impl(base, flags); + for (;;) { +rescan: + /* Make sure we have some events. */ + if (TAILQ_EMPTY(&base->events)) { + rc = 1; + break; + } + + /* Call backend to setup the active queue. */ + TAILQ_INIT(&base->active); + rc = sudo_ev_loop_impl(base, flags); + if (rc == -1) { + if (errno == EINTR || errno == ENOMEM) + continue; + break; + } + + /* + * Service each event in the active queue. + * We store the current event pointer in the base so that + * it can be cleared by sudo_ev_del(). This prevents a use + * after free if the callback frees its own event. + */ + TAILQ_FOREACH_SAFE(base->cur, &base->active, active_entries, base->pending) { + if (!ISSET(base->cur->events, SUDO_EV_PERSIST)) + SET(base->cur->flags, SUDO_EV_DELETE); + base->cur->callback(base->cur->fd, base->cur->revents, + base->cur->closure); + if (base->cur != NULL) { + CLR(base->cur->flags, SUDO_EV_ACTIVE); + if (ISSET(base->cur->flags, SUDO_EV_DELETE)) + sudo_ev_del(base, base->cur); + } + if (ISSET(base->flags, SUDO_EVBASE_LOOPBREAK)) { + /* stop processing events immediately */ + base->flags |= SUDO_EVBASE_GOT_BREAK; + base->pending = NULL; + goto done; + } + if (ISSET(base->flags, SUDO_EVBASE_LOOPCONT)) { + /* rescan events and start polling again */ + CLR(base->flags, SUDO_EVBASE_LOOPCONT); + base->pending = NULL; + goto rescan; + } + } + base->pending = NULL; + if (ISSET(base->flags, SUDO_EVBASE_LOOPEXIT)) { + /* exit loop after once through */ + base->flags |= SUDO_EVBASE_GOT_EXIT; + goto done; + } + if (flags & (SUDO_EVLOOP_ONCE | SUDO_EVLOOP_NONBLOCK)) + break; + } +done: base->flags &= SUDO_EVBASE_GOT_MASK; debug_return_int(rc); } diff --git a/common/event_poll.c b/common/event_poll.c index 4f153b5a0..d6bd7aa07 100644 --- a/common/event_poll.c +++ b/common/event_poll.c @@ -138,58 +138,31 @@ sudo_ev_loop_impl(struct sudo_event_base *base, int flags) int nready; debug_decl(sudo_ev_loop_impl, SUDO_DEBUG_EVENT) -rescan: - while (base->pfd_high != -1) { - nready = poll(base->pfds, base->pfd_high + 1, timeout); - sudo_debug_printf(SUDO_DEBUG_INFO, "%s: %d fds ready", - __func__, nready); - switch (nready) { - case -1: - if (errno == EINTR || errno == ENOMEM) - continue; - debug_return_int(-1); - case 0: - /* timeout or no events */ - break; - default: - /* Service each event that fired. */ - TAILQ_FOREACH_SAFE(ev, &base->events, entries, base->pending) { - if (ev->pfd_idx != -1 && base->pfds[ev->pfd_idx].revents) { - int what = 0; - if (base->pfds[ev->pfd_idx].revents & (POLLIN|POLLHUP|POLLNVAL|POLLERR)) - what |= (ev->events & SUDO_EV_READ); - if (base->pfds[ev->pfd_idx].revents & (POLLOUT|POLLHUP|POLLNVAL|POLLERR)) - what |= (ev->events & SUDO_EV_WRITE); - if (!ISSET(ev->events, SUDO_EV_PERSIST)) - SET(ev->events, SUDO_EV_DELETE); - ev->callback(ev->fd, what, ev->closure); - if (ISSET(ev->events, SUDO_EV_DELETE)) - sudo_ev_del(base, ev); - if (ISSET(base->flags, SUDO_EVBASE_LOOPBREAK)) { - /* stop processing events immediately */ - base->flags |= SUDO_EVBASE_GOT_BREAK; - base->pending = NULL; - goto done; - } - if (ISSET(base->flags, SUDO_EVBASE_LOOPCONT)) { - /* rescan events and start polling again */ - CLR(base->flags, SUDO_EVBASE_LOOPCONT); - base->pending = NULL; - goto rescan; - } - } + nready = poll(base->pfds, base->pfd_high + 1, timeout); + sudo_debug_printf(SUDO_DEBUG_INFO, "%s: %d fds ready", __func__, nready); + switch (nready) { + case -1: + /* error or interrupted by signal */ + debug_return_int(-1); + case 0: + /* timeout or no events */ + break; + default: + /* Activate each event that fired. */ + TAILQ_FOREACH(ev, &base->events, entries) { + if (ev->pfd_idx != -1 && base->pfds[ev->pfd_idx].revents) { + int what = 0; + if (base->pfds[ev->pfd_idx].revents & (POLLIN|POLLHUP|POLLNVAL|POLLERR)) + what |= (ev->events & SUDO_EV_READ); + if (base->pfds[ev->pfd_idx].revents & (POLLOUT|POLLHUP|POLLNVAL|POLLERR)) + what |= (ev->events & SUDO_EV_WRITE); + /* Make event active. */ + ev->revents = what; + SET(ev->flags, SUDO_EV_ACTIVE); + TAILQ_INSERT_TAIL(&base->active, ev, active_entries); } - base->pending = NULL; - if (ISSET(base->flags, SUDO_EVBASE_LOOPEXIT)) { - /* exit loop after once through */ - base->flags |= SUDO_EVBASE_GOT_EXIT; - goto done; - } - break; } - if (flags & (SUDO_EVLOOP_ONCE | SUDO_EVLOOP_NONBLOCK)) - break; + break; } -done: debug_return_int(0); } diff --git a/common/event_select.c b/common/event_select.c index dfb8b70a6..1410f3488 100644 --- a/common/event_select.c +++ b/common/event_select.c @@ -62,7 +62,6 @@ sudo_ev_base_alloc_impl(struct sudo_event_base *base) debug_decl(sudo_ev_base_alloc_impl, SUDO_DEBUG_EVENT) base->maxfd = NFDBITS - 1; - base->nevents = 0; base->readfds = ecalloc(1, sizeof(fd_mask)); base->writefds = ecalloc(1, sizeof(fd_mask)); @@ -92,7 +91,6 @@ sudo_ev_add_impl(struct sudo_event_base *base, struct sudo_event *ev) base->writefds = ecalloc(n, sizeof(fd_mask)); base->maxfd = (n * NFDBITS) - 1; } - base->nevents++; debug_return_int(0); } @@ -105,7 +103,6 @@ sudo_ev_del_impl(struct sudo_event_base *base, struct sudo_event *ev) /* Remove from readfds and writefds and adjust event count. */ FD_CLR(ev->fd, base->readfds); FD_CLR(ev->fd, base->writefds); - base->nevents--; debug_return_int(0); } @@ -115,7 +112,7 @@ sudo_ev_loop_impl(struct sudo_event_base *base, int flags) { struct timeval tv, *timeout; struct sudo_event *ev; - int nready; + int nready, highfd = 0; debug_decl(sudo_ev_loop, SUDO_DEBUG_EVENT) if (ISSET(flags, SUDO_EVLOOP_NONBLOCK)) { @@ -126,80 +123,52 @@ sudo_ev_loop_impl(struct sudo_event_base *base, int flags) timeout = NULL; } -rescan: - while (base->nevents != 0) { - int highfd = 0; - /* For select we need to redo readfds and writefds each time. */ - memset(base->readfds, 0, howmany(base->maxfd + 1, NFDBITS) * sizeof(fd_mask)); - memset(base->writefds, 0, howmany(base->maxfd + 1, NFDBITS) * sizeof(fd_mask)); - TAILQ_FOREACH(ev, &base->events, entries) { - if (ISSET(ev->events, SUDO_EV_READ)) { - sudo_debug_printf(SUDO_DEBUG_DEBUG, "%s: added fd %d to readfs", - __func__, ev->fd); - FD_SET(ev->fd, base->readfds); - if (ev->fd > highfd) - highfd = ev->fd; - } - if (ISSET(ev->events, SUDO_EV_WRITE)) { - sudo_debug_printf(SUDO_DEBUG_DEBUG, "%s: added fd %d to writefds", - __func__, ev->fd); - FD_SET(ev->fd, base->writefds); - if (ev->fd > highfd) - highfd = ev->fd; - } + /* For select we need to redo readfds and writefds each time. */ + memset(base->readfds, 0, howmany(base->maxfd + 1, NFDBITS) * sizeof(fd_mask)); + memset(base->writefds, 0, howmany(base->maxfd + 1, NFDBITS) * sizeof(fd_mask)); + TAILQ_FOREACH(ev, &base->events, entries) { + if (ISSET(ev->events, SUDO_EV_READ)) { + sudo_debug_printf(SUDO_DEBUG_DEBUG, "%s: added fd %d to readfs", + __func__, ev->fd); + FD_SET(ev->fd, base->readfds); + if (ev->fd > highfd) + highfd = ev->fd; } - sudo_debug_printf(SUDO_DEBUG_DEBUG, "%s: select high fd %d", - __func__, highfd); - nready = select(highfd + 1, base->readfds, base->writefds, NULL, timeout); - sudo_debug_printf(SUDO_DEBUG_INFO, "%s: %d fds ready", - __func__, nready); - switch (nready) { - case -1: - if (errno == EINTR || errno == ENOMEM) - continue; - debug_return_int(-1); - case 0: - /* timeout or no events */ - break; - default: - /* Service each event that fired. */ - TAILQ_FOREACH_SAFE(ev, &base->events, entries, base->pending) { - int what = 0; - if (FD_ISSET(ev->fd, base->readfds)) - what |= (ev->events & SUDO_EV_READ); - if (FD_ISSET(ev->fd, base->writefds)) - what |= (ev->events & SUDO_EV_WRITE); - if (what != 0) { - if (!ISSET(ev->events, SUDO_EV_PERSIST)) - SET(ev->events, SUDO_EV_DELETE); - ev->callback(ev->fd, what, ev->closure); - if (ISSET(ev->events, SUDO_EV_DELETE)) - sudo_ev_del(base, ev); - if (ISSET(base->flags, SUDO_EVBASE_LOOPBREAK)) { - /* stop processing events immediately */ - base->flags |= SUDO_EVBASE_GOT_BREAK; - base->pending = NULL; - goto done; - } - if (ISSET(base->flags, SUDO_EVBASE_LOOPCONT)) { - /* rescan events and start polling again */ - CLR(base->flags, SUDO_EVBASE_LOOPCONT); - base->pending = NULL; - goto rescan; - } - } - } - base->pending = NULL; - if (ISSET(base->flags, SUDO_EVBASE_LOOPEXIT)) { - /* exit loop after once through */ - base->flags |= SUDO_EVBASE_GOT_EXIT; - goto done; - } - break; + if (ISSET(ev->events, SUDO_EV_WRITE)) { + sudo_debug_printf(SUDO_DEBUG_DEBUG, "%s: added fd %d to writefds", + __func__, ev->fd); + FD_SET(ev->fd, base->writefds); + if (ev->fd > highfd) + highfd = ev->fd; } - if (flags & (SUDO_EVLOOP_ONCE | SUDO_EVLOOP_NONBLOCK)) - break; } -done: + sudo_debug_printf(SUDO_DEBUG_DEBUG, "%s: select high fd %d", + __func__, highfd); + nready = select(highfd + 1, base->readfds, base->writefds, NULL, timeout); + sudo_debug_printf(SUDO_DEBUG_INFO, "%s: %d fds ready", __func__, nready); + switch (nready) { + case -1: + /* error or interrupted by signal */ + debug_return_int(-1); + case 0: + /* timeout or no events */ + break; + default: + /* Activate each event that fired. */ + TAILQ_FOREACH(ev, &base->events, entries) { + int what = 0; + if (FD_ISSET(ev->fd, base->readfds)) + what |= (ev->events & SUDO_EV_READ); + if (FD_ISSET(ev->fd, base->writefds)) + what |= (ev->events & SUDO_EV_WRITE); + if (what != 0) { + /* Make event active. */ + ev->revents = what; + SET(ev->flags, SUDO_EV_ACTIVE); + TAILQ_INSERT_TAIL(&base->active, ev, active_entries); + } + } + break; + } debug_return_int(0); } diff --git a/include/sudo_event.h b/include/sudo_event.h index bc1150b85..84490643c 100644 --- a/include/sudo_event.h +++ b/include/sudo_event.h @@ -20,10 +20,13 @@ #include "queue.h" /* Event types */ -#define SUDO_EV_READ 0x01 /* fire when readable */ -#define SUDO_EV_WRITE 0x02 /* fire when writable */ -#define SUDO_EV_PERSIST 0x04 /* persist until deleted */ -#define SUDO_EV_DELETE 0x08 /* deletion pending */ +#define SUDO_EV_READ 0x01 /* fire when readable */ +#define SUDO_EV_WRITE 0x02 /* fire when writable */ +#define SUDO_EV_PERSIST 0x04 /* persist until deleted */ + +/* Event flags (internal) */ +#define SUDO_EV_ACTIVE 0x01 /* event is on the active queue */ +#define SUDO_EV_DELETE 0x02 /* deletion pending */ /* Event loop flags */ #define SUDO_EVLOOP_ONCE 0x01 /* Only run once through the loop */ @@ -42,10 +45,13 @@ typedef void (*sudo_ev_callback_t)(int fd, int what, void *closure); /* Member of struct sudo_event_base. */ struct sudo_event { TAILQ_ENTRY(sudo_event) entries; + TAILQ_ENTRY(sudo_event) active_entries; struct sudo_event_base *base; /* base this event belongs to */ int fd; /* fd we are interested in */ - short events; /* SUDO_EV_* flags */ - short pfd_idx; /* index into pfds array */ + short events; /* SUDO_EV_* flags (in) */ + short revents; /* SUDO_EV_* flags (out) */ + short flags; /* internal event flags */ + short pfd_idx; /* index into pfds array (XXX) */ sudo_ev_callback_t callback;/* user-provided callback */ void *closure; /* user-provided data pointer */ }; @@ -53,9 +59,10 @@ struct sudo_event { TAILQ_HEAD(sudo_event_list, sudo_event); struct sudo_event_base { - struct sudo_event_list events; /* tail queue of events */ - /* XXX - also list of active events and timed events */ - struct sudo_event *pending; /* next event to be run in the event loop XXX */ + struct sudo_event_list events; /* tail queue of all events */ + struct sudo_event_list active; /* tail queue of active events */ + struct sudo_event *cur; /* current active event being serviced */ + struct sudo_event *pending; /* next active event to be serviced */ #ifdef HAVE_POLL struct pollfd *pfds; /* array of struct pollfd */ int pfd_max; /* size of the pfds array */ @@ -65,7 +72,6 @@ struct sudo_event_base { fd_set *readfds; /* read I/O descriptor set */ fd_set *writefds; /* write I/O descriptor set */ int maxfd; /* max fd we can store in readfds/writefds */ - int nevents; /* number of events in the list */ #endif /* HAVE_POLL */ unsigned int flags; /* SUDO_EVBASE_* */ };