From b04c49dbd312e2effdada1b3e9fcbd63eb26149c Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Mon, 9 May 2016 10:53:20 -0600 Subject: [PATCH] Break up io_callback() into read_callback() and write_callback() to make it clear that we can't get an event with both read and write set. --- src/exec_pty.c | 223 +++++++++++++++++++++++++------------------------ 1 file changed, 115 insertions(+), 108 deletions(-) diff --git a/src/exec_pty.c b/src/exec_pty.c index 602eac303..369f2b543 100644 --- a/src/exec_pty.c +++ b/src/exec_pty.c @@ -537,130 +537,137 @@ terminate_command(pid_t pid, bool use_pgrp) } /* - * Read/write an iobuf that is ready. + * Read an iobuf that is ready. */ static void -io_callback(int fd, int what, void *v) +read_callback(int fd, int what, void *v) { struct io_buffer *iob = v; struct sudo_event_base *evbase; int n; - debug_decl(io_callback, SUDO_DEBUG_EXEC); + debug_decl(read_callback, SUDO_DEBUG_EXEC); - if (ISSET(what, SUDO_EV_WRITE)) { - evbase = sudo_ev_get_base(iob->wevent); - do { - n = write(fd, iob->buf + iob->off, iob->len - iob->off); - } while (n == -1 && errno == EINTR); - if (n == -1) { - switch (errno) { - case EPIPE: - case ENXIO: - case EIO: - case EBADF: - /* other end of pipe closed or pty revoked */ - sudo_debug_printf(SUDO_DEBUG_INFO, - "unable to write %d bytes to fd %d", - iob->len - iob->off, fd); - /* Close reader if there is one. */ - if (iob->revent != NULL) { - safe_close(sudo_ev_get_fd(iob->revent)); - ev_free_by_fd(evbase, sudo_ev_get_fd(iob->revent)); - } - CLR(what, SUDO_EV_READ); - safe_close(fd); - ev_free_by_fd(evbase, fd); - break; - case EAGAIN: - /* not an error */ - break; - default: -#if 0 /* XXX -- how to set cstat? stash in iobufs instead? */ - if (cstat != NULL) { - cstat->type = CMD_ERRNO; - cstat->val = errno; - } -#endif /* XXX */ - sudo_debug_printf(SUDO_DEBUG_ERROR, - "error writing fd %d: %s", fd, strerror(errno)); - sudo_ev_loopbreak(evbase); + evbase = sudo_ev_get_base(iob->revent); + do { + n = read(fd, iob->buf + iob->len, sizeof(iob->buf) - iob->len); + } while (n == -1 && errno == EINTR); + switch (n) { + case -1: + if (errno == EAGAIN) break; + /* treat read error as fatal and close the fd */ + sudo_debug_printf(SUDO_DEBUG_ERROR, + "error reading fd %d: %s", fd, strerror(errno)); + /* FALLTHROUGH */ + case 0: + /* got EOF or pty has gone away */ + if (n == 0) { + sudo_debug_printf(SUDO_DEBUG_INFO, + "read EOF from fd %d", fd); } - } else { - sudo_debug_printf(SUDO_DEBUG_INFO, - "wrote %d bytes to fd %d", n, fd); - iob->off += n; - /* Reset buffer if fully consumed. */ - if (iob->off == iob->len) { + safe_close(fd); + ev_free_by_fd(evbase, fd); + /* If writer already consumed the buffer, close it too. */ + if (iob->wevent != NULL && iob->off == iob->len) { + safe_close(sudo_ev_get_fd(iob->wevent)); + ev_free_by_fd(evbase, sudo_ev_get_fd(iob->wevent)); iob->off = iob->len = 0; - /* Forward the EOF from reader to writer. */ - if (iob->revent == NULL) { - CLR(what, SUDO_EV_READ); - safe_close(fd); - ev_free_by_fd(evbase, fd); - } } - /* Re-enable writer if buffer is not empty. */ - if (iob->len > iob->off) { + break; + default: + sudo_debug_printf(SUDO_DEBUG_INFO, + "read %d bytes from fd %d", n, fd); + if (!iob->action(iob->buf + iob->len, n, iob)) + terminate_command(cmnd_pid, true); + iob->len += n; + /* Enable writer if not /dev/tty or we are foreground pgrp. */ + if (iob->wevent != NULL && + (foreground || !USERTTY_EVENT(iob->wevent))) { if (sudo_ev_add(evbase, iob->wevent, NULL, false) == -1) sudo_fatal(U_("unable to add event to queue")); } - /* Enable reader if buffer is not full. */ - if (iob->revent != NULL && - (ttymode == TERM_RAW || !USERTTY_EVENT(iob->revent))) { - if (iob->len != sizeof(iob->buf)) { - if (sudo_ev_add(evbase, iob->revent, NULL, false) == -1) - sudo_fatal(U_("unable to add event to queue")); - } + /* Re-enable reader if buffer is not full. */ + if (iob->len != sizeof(iob->buf)) { + if (sudo_ev_add(evbase, iob->revent, NULL, false) == -1) + sudo_fatal(U_("unable to add event to queue")); } - } + break; } - if (ISSET(what, SUDO_EV_READ)) { - evbase = sudo_ev_get_base(iob->revent); - do { - n = read(fd, iob->buf + iob->len, sizeof(iob->buf) - iob->len); - } while (n == -1 && errno == EINTR); - switch (n) { - case -1: - if (errno == EAGAIN) - break; - /* treat read error as fatal and close the fd */ - sudo_debug_printf(SUDO_DEBUG_ERROR, - "error reading fd %d: %s", fd, strerror(errno)); - /* FALLTHROUGH */ - case 0: - /* got EOF or pty has gone away */ - if (n == 0) { - sudo_debug_printf(SUDO_DEBUG_INFO, - "read EOF from fd %d", fd); - } +} + +/* + * Write an iobuf that is ready. + */ +static void +write_callback(int fd, int what, void *v) +{ + struct io_buffer *iob = v; + struct sudo_event_base *evbase; + int n; + debug_decl(write_callback, SUDO_DEBUG_EXEC); + + evbase = sudo_ev_get_base(iob->wevent); + do { + n = write(fd, iob->buf + iob->off, iob->len - iob->off); + } while (n == -1 && errno == EINTR); + if (n == -1) { + switch (errno) { + case EPIPE: + case ENXIO: + case EIO: + case EBADF: + /* other end of pipe closed or pty revoked */ + sudo_debug_printf(SUDO_DEBUG_INFO, + "unable to write %d bytes to fd %d", + iob->len - iob->off, fd); + /* Close reader if there is one. */ + if (iob->revent != NULL) { + safe_close(sudo_ev_get_fd(iob->revent)); + ev_free_by_fd(evbase, sudo_ev_get_fd(iob->revent)); + } + safe_close(fd); + ev_free_by_fd(evbase, fd); + break; + case EAGAIN: + /* not an error */ + break; + default: +#if 0 /* XXX -- how to set cstat? stash in iobufs instead? */ + if (cstat != NULL) { + cstat->type = CMD_ERRNO; + cstat->val = errno; + } +#endif /* XXX */ + sudo_debug_printf(SUDO_DEBUG_ERROR, + "error writing fd %d: %s", fd, strerror(errno)); + sudo_ev_loopbreak(evbase); + break; + } + } else { + sudo_debug_printf(SUDO_DEBUG_INFO, + "wrote %d bytes to fd %d", n, fd); + iob->off += n; + /* Reset buffer if fully consumed. */ + if (iob->off == iob->len) { + iob->off = iob->len = 0; + /* Forward the EOF from reader to writer. */ + if (iob->revent == NULL) { safe_close(fd); ev_free_by_fd(evbase, fd); - /* If writer already consumed the buffer, close it too. */ - if (iob->wevent != NULL && iob->off == iob->len) { - safe_close(sudo_ev_get_fd(iob->wevent)); - ev_free_by_fd(evbase, sudo_ev_get_fd(iob->wevent)); - iob->off = iob->len = 0; - } - break; - default: - sudo_debug_printf(SUDO_DEBUG_INFO, - "read %d bytes from fd %d", n, fd); - if (!iob->action(iob->buf + iob->len, n, iob)) - terminate_command(cmnd_pid, true); - iob->len += n; - /* Enable writer if not /dev/tty or we are foreground pgrp. */ - if (iob->wevent != NULL && - (foreground || !USERTTY_EVENT(iob->wevent))) { - if (sudo_ev_add(evbase, iob->wevent, NULL, false) == -1) - sudo_fatal(U_("unable to add event to queue")); - } - /* Re-enable reader if buffer is not full. */ - if (iob->len != sizeof(iob->buf)) { - if (sudo_ev_add(evbase, iob->revent, NULL, false) == -1) - sudo_fatal(U_("unable to add event to queue")); - } - break; + } + } + /* Re-enable writer if buffer is not empty. */ + if (iob->len > iob->off) { + if (sudo_ev_add(evbase, iob->wevent, NULL, false) == -1) + sudo_fatal(U_("unable to add event to queue")); + } + /* Enable reader if buffer is not full. */ + if (iob->revent != NULL && + (ttymode == TERM_RAW || !USERTTY_EVENT(iob->revent))) { + if (iob->len != sizeof(iob->buf)) { + if (sudo_ev_add(evbase, iob->revent, NULL, false) == -1) + sudo_fatal(U_("unable to add event to queue")); + } } } } @@ -684,8 +691,8 @@ io_buf_new(int rfd, int wfd, bool (*action)(const char *, unsigned int, struct i /* Allocate and add to head of list. */ if ((iob = malloc(sizeof(*iob))) == NULL) sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory")); - iob->revent = sudo_ev_alloc(rfd, SUDO_EV_READ, io_callback, iob); - iob->wevent = sudo_ev_alloc(wfd, SUDO_EV_WRITE, io_callback, iob); + iob->revent = sudo_ev_alloc(rfd, SUDO_EV_READ, read_callback, iob); + iob->wevent = sudo_ev_alloc(wfd, SUDO_EV_WRITE, write_callback, iob); iob->len = 0; iob->off = 0; iob->action = action; -- 2.40.0