From 5787da7b21ee187dd519658932dae1d9cf98601d Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Mon, 13 Jun 2022 14:59:00 -0600 Subject: [PATCH 1/6] Quiet a compiler warning on macOS. The getgrouplist() groups array on macOS is int * instead of gid_t *. --- lib/util/getgrouplist.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/util/getgrouplist.c b/lib/util/getgrouplist.c index 96e91089e..125d49460 100644 --- a/lib/util/getgrouplist.c +++ b/lib/util/getgrouplist.c @@ -66,7 +66,11 @@ int sudo_getgrouplist2_v1(const char *name, GETGROUPS_T basegid, GETGROUPS_T **groupsp, int *ngroupsp) { +#ifdef __APPLE__ + int *groups = (int *)*groupsp; +#else GETGROUPS_T *groups = *groupsp; +#endif int ngroups; #ifndef HAVE_GETGROUPLIST_2 int grpsize, tries; From 49c27f5278315491dd70fadd5f164277e4d41eff Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Tue, 14 Jun 2022 13:10:13 -0600 Subject: [PATCH 2/6] log_exit_status: make local variables match struct evlog members. --- plugins/sudoers/logging.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/plugins/sudoers/logging.c b/plugins/sudoers/logging.c index d681b8253..286c6aee8 100644 --- a/plugins/sudoers/logging.c +++ b/plugins/sudoers/logging.c @@ -573,15 +573,15 @@ log_allowed(struct eventlog *evlog) } bool -log_exit_status(int exit_status) +log_exit_status(int status) { struct eventlog evlog; int evl_flags = 0; - int ecode = 0; + int exit_value = 0; int oldlocale; struct timespec run_time; char sigbuf[SIG2STR_MAX]; - char *signame = NULL; + char *signal_name = NULL; bool dumped_core = false; bool ret = true; debug_decl(log_exit_status, SUDOERS_DEBUG_LOGGING); @@ -594,17 +594,17 @@ log_exit_status(int exit_status) } sudo_timespecsub(&run_time, &sudo_user.submit_time, &run_time); - if (WIFEXITED(exit_status)) { - ecode = WEXITSTATUS(exit_status); - } else if (WIFSIGNALED(exit_status)) { - int signo = WTERMSIG(exit_status); + if (WIFEXITED(status)) { + exit_value = WEXITSTATUS(status); + } else if (WIFSIGNALED(status)) { + int signo = WTERMSIG(status); if (signo <= 0 || sig2str(signo, sigbuf) == -1) (void)snprintf(sigbuf, sizeof(sigbuf), "%d", signo); - signame = sigbuf; - ecode = signo | 128; - dumped_core = WCOREDUMP(exit_status); + signal_name = sigbuf; + exit_value = signo | 128; + dumped_core = WCOREDUMP(status); } else { - sudo_warnx("invalid exit status 0x%x", exit_status); + sudo_warnx("invalid exit status 0x%x", status); ret = false; goto done; } @@ -619,8 +619,8 @@ log_exit_status(int exit_status) SET(evl_flags, EVLOG_MAIL_ONLY); } evlog.run_time = run_time; - evlog.exit_value = ecode; - evlog.signal_name = signame; + evlog.exit_value = exit_value; + evlog.signal_name = signal_name; evlog.dumped_core = dumped_core; if (!eventlog_exit(&evlog, evl_flags)) ret = false; From 8829c028d357831238fac708204f868be97e526b Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Mon, 20 Jun 2022 14:58:06 -0600 Subject: [PATCH 3/6] Add debug printfs when send/recv return EAGAIN or EINTR. These are not actually errors but can help gain insight into what is going on and, in the case of EAGAIN, whether or not there may be a kernel resource starvation problem. --- src/exec_intercept.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/exec_intercept.c b/src/exec_intercept.c index 3f7064aea..659f457e0 100644 --- a/src/exec_intercept.c +++ b/src/exec_intercept.c @@ -560,8 +560,12 @@ intercept_read(int fd, struct intercept_closure *closure) case false: goto done; default: - if (errno == EINTR || errno == EAGAIN) + if (errno == EINTR || errno == EAGAIN) { debug_return_bool(true); + sudo_debug_printf( + SUDO_DEBUG_WARN|SUDO_DEBUG_ERRNO|SUDO_DEBUG_LINENO, + "reading intercept token"); + } sudo_warn("recv"); goto done; } @@ -574,8 +578,12 @@ intercept_read(int fd, struct intercept_closure *closure) nread = recv(fd, &req_len, sizeof(req_len), 0); if (nread != sizeof(req_len)) { if (nread == -1) { - if (errno == EINTR || errno == EAGAIN) + if (errno == EINTR || errno == EAGAIN) { + sudo_debug_printf( + SUDO_DEBUG_WARN|SUDO_DEBUG_ERRNO|SUDO_DEBUG_LINENO, + "reading intercept message size"); debug_return_bool(true); + } sudo_warn("recv"); } goto done; @@ -605,8 +613,12 @@ intercept_read(int fd, struct intercept_closure *closure) /* EOF, other side must have exited. */ goto done; case -1: - if (errno == EINTR || errno == EAGAIN) + if (errno == EINTR || errno == EAGAIN) { + sudo_debug_printf( + SUDO_DEBUG_WARN|SUDO_DEBUG_ERRNO|SUDO_DEBUG_LINENO, + "reading intercept message"); debug_return_bool(true); + } sudo_warn("recv"); goto done; default: @@ -835,8 +847,12 @@ intercept_write(int fd, struct intercept_closure *closure) nwritten = send(fd, closure->buf + closure->off, closure->len - closure->off, 0); if (nwritten == -1) { - if (errno == EINTR || errno == EAGAIN) + if (errno == EINTR || errno == EAGAIN) { + sudo_debug_printf( + SUDO_DEBUG_WARN|SUDO_DEBUG_ERRNO|SUDO_DEBUG_LINENO, + "writing intercept message"); debug_return_bool(true); + } sudo_warn("send"); goto done; } From b10201bdc4b0e4f14e88eac8abdd8fc92280e3a2 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Mon, 20 Jun 2022 15:02:11 -0600 Subject: [PATCH 4/6] Use blocking I/O when talking to the sudo process. Also check for EAGAIN/EINTR when reading the message size. Fixes a problem seen on AIX where recv_intercept_response() could fail unexpectedly. Bug #1034. --- src/sudo_intercept_common.c | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/src/sudo_intercept_common.c b/src/sudo_intercept_common.c index 0702b8c81..2a89cc64f 100644 --- a/src/sudo_intercept_common.c +++ b/src/sudo_intercept_common.c @@ -136,16 +136,29 @@ recv_intercept_response(int fd) debug_decl(recv_intercept_response, SUDO_DEBUG_EXEC); /* Read message size (uint32_t in host byte order). */ - nread = recv(fd, &res_len, sizeof(res_len), 0); - if ((size_t)nread != sizeof(res_len)) { - if (nread == 0) { + for (;;) { + nread = recv(fd, &res_len, sizeof(res_len), 0); + if (nread == ssizeof(res_len)) + break; + switch (nread) { + case 0: sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, "unexpected EOF reading response size"); - } else { - sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, + break; + case -1: + if (errno == EINTR) + continue; + sudo_debug_printf( + SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, "error reading response size"); + break; + default: + sudo_debug_printf( + SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, + "error reading response size: short read"); + break; } - goto done; + goto done; } if (res_len > MESSAGE_SIZE_MAX) { sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, @@ -169,7 +182,8 @@ recv_intercept_response(int fd) case -1: if (errno == EINTR) continue; - sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, + sudo_debug_printf( + SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, "error reading response"); goto done; default: @@ -199,7 +213,7 @@ sudo_interposer_init(void) { InterceptResponse *res = NULL; static bool initialized; - int fd = -1; + int flags, fd = -1; char **p; debug_decl(sudo_interposer_init, SUDO_DEBUG_EXEC); @@ -239,6 +253,13 @@ sudo_interposer_init(void) goto done; } + /* + * We don't want to use non-blocking I/O. + */ + flags = fcntl(fd, F_GETFL, 0); + if (flags != -1) + (void)fcntl(fd, F_SETFL, flags & ~O_NONBLOCK); + /* * Send InterceptHello message to over the fd. */ From 332a6afe77b914220abee9be5c455ab4c5c015e9 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Mon, 20 Jun 2022 16:22:29 -0600 Subject: [PATCH 5/6] Set TCP_NODELAY on the socket used for intercept IPC to reduce latency. On some systems, Nagle's algorithm was delaying receipt of the data, causing commands with intercept or log_subcmds to run slowly. Related to Bug #1034. --- src/exec_intercept.c | 6 +++++- src/sudo_intercept_common.c | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/exec_intercept.c b/src/exec_intercept.c index 659f457e0..6560d93cf 100644 --- a/src/exec_intercept.c +++ b/src/exec_intercept.c @@ -25,6 +25,7 @@ #include #include +#include #if defined(HAVE_STDINT_H) # include @@ -946,7 +947,7 @@ intercept_accept_cb(int fd, int what, void *v) struct sudo_event_base *evbase = sudo_ev_get_base(&closure->ev); struct sockaddr_in sin; socklen_t sin_len = sizeof(sin); - int client_sock, flags; + int client_sock, flags, on = 1; debug_decl(intercept_accept_cb, SUDO_DEBUG_EXEC); if (closure->state != RECV_CONNECTION) { @@ -967,6 +968,9 @@ intercept_accept_cb(int fd, int what, void *v) if (flags != -1) (void)fcntl(client_sock, F_SETFL, flags | O_NONBLOCK); + /* Send data immediately, we need low latency IPC. */ + (void)setsockopt(client_sock, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)); + /* * Create a new intercept closure and register an event for client_sock. */ diff --git a/src/sudo_intercept_common.c b/src/sudo_intercept_common.c index 2a89cc64f..e6431882b 100644 --- a/src/sudo_intercept_common.c +++ b/src/sudo_intercept_common.c @@ -26,6 +26,7 @@ #include #include #include +#include #if defined(HAVE_STDINT_H) # include @@ -355,6 +356,7 @@ static int intercept_connect(void) { int sock = -1; + int on = 1; struct sockaddr_in sin; debug_decl(command_allowed, SUDO_DEBUG_EXEC); @@ -374,6 +376,9 @@ intercept_connect(void) goto done; } + /* Send data immediately, we need low latency IPC. */ + (void)setsockopt(sock, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)); + if (connect(sock, (struct sockaddr *)&sin, sizeof(sin)) == -1) { sudo_warn("connect"); close(sock); From 01a9e5a157156db9b8d281892bed1569aab10ee5 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Mon, 20 Jun 2022 16:58:03 -0600 Subject: [PATCH 6/6] Sudo 1.9.11p3 --- NEWS | 11 +++++++++++ configure | 18 +++++++++--------- configure.ac | 2 +- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 36c0cd285..10744d326 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,14 @@ +What's new in Sudo 1.9.11p3 + + * Fixed "connection reset" errors on AIX when running shell scripts + with the "intercept" or "log_subcmds" sudoers options enabled. + Bug #1034. + + * Fixed very slow execution of shell scripts when the "intercept" + or "log_subcmds" sudoers options are set on systems that enable + Nagle's algorithm on the loopback device, such as AIX. + Bug #1034. + What's new in Sudo 1.9.11p2 * Fixed a compilation error on Linux/x86_64 with the x32 ABI. diff --git a/configure b/configure index 677458542..2363bb148 100755 --- a/configure +++ b/configure @@ -1,6 +1,6 @@ #! /bin/sh # Guess values for system-dependent variables and create Makefiles. -# Generated by GNU Autoconf 2.71 for sudo 1.9.11p2. +# Generated by GNU Autoconf 2.71 for sudo 1.9.11p3. # # Report bugs to . # @@ -621,8 +621,8 @@ MAKEFLAGS= # Identity of this package. PACKAGE_NAME='sudo' PACKAGE_TARNAME='sudo' -PACKAGE_VERSION='1.9.11p2' -PACKAGE_STRING='sudo 1.9.11p2' +PACKAGE_VERSION='1.9.11p3' +PACKAGE_STRING='sudo 1.9.11p3' PACKAGE_BUGREPORT='https://bugzilla.sudo.ws/' PACKAGE_URL='' @@ -1640,7 +1640,7 @@ if test "$ac_init_help" = "long"; then # Omit some internal or obsolete options to make the list less imposing. # This message is too long to be a string in the A/UX 3.1 sh. cat <<_ACEOF -\`configure' configures sudo 1.9.11p2 to adapt to many kinds of systems. +\`configure' configures sudo 1.9.11p3 to adapt to many kinds of systems. Usage: $0 [OPTION]... [VAR=VALUE]... @@ -1706,7 +1706,7 @@ fi if test -n "$ac_init_help"; then case $ac_init_help in - short | recursive ) echo "Configuration of sudo 1.9.11p2:";; + short | recursive ) echo "Configuration of sudo 1.9.11p3:";; esac cat <<\_ACEOF @@ -1996,7 +1996,7 @@ fi test -n "$ac_init_help" && exit $ac_status if $ac_init_version; then cat <<\_ACEOF -sudo configure 1.9.11p2 +sudo configure 1.9.11p3 generated by GNU Autoconf 2.71 Copyright (C) 2021 Free Software Foundation, Inc. @@ -2653,7 +2653,7 @@ cat >config.log <<_ACEOF This file contains any messages produced by compilers while running configure, to aid debugging if configure makes a mistake. -It was created by sudo $as_me 1.9.11p2, which was +It was created by sudo $as_me 1.9.11p3, which was generated by GNU Autoconf 2.71. Invocation command line was $ $0$ac_configure_args_raw @@ -33050,7 +33050,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1 # report actual input values of CONFIG_FILES etc. instead of their # values after options handling. ac_log=" -This file was extended by sudo $as_me 1.9.11p2, which was +This file was extended by sudo $as_me 1.9.11p3, which was generated by GNU Autoconf 2.71. Invocation command line was CONFIG_FILES = $CONFIG_FILES @@ -33118,7 +33118,7 @@ ac_cs_config_escaped=`printf "%s\n" "$ac_cs_config" | sed "s/^ //; s/'/'\\\\\\\\ cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1 ac_cs_config='$ac_cs_config_escaped' ac_cs_version="\\ -sudo config.status 1.9.11p2 +sudo config.status 1.9.11p3 configured by $0, generated by GNU Autoconf 2.71, with options \\"\$ac_cs_config\\" diff --git a/configure.ac b/configure.ac index 400700bbe..9f845eddd 100644 --- a/configure.ac +++ b/configure.ac @@ -18,7 +18,7 @@ dnl ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF dnl OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. dnl AC_PREREQ([2.70]) -AC_INIT([sudo], [1.9.11p2], [https://bugzilla.sudo.ws/], [sudo]) +AC_INIT([sudo], [1.9.11p3], [https://bugzilla.sudo.ws/], [sudo]) AC_CONFIG_HEADERS([config.h pathnames.h]) AC_CONFIG_SRCDIR([src/sudo.c]) AC_CONFIG_AUX_DIR([scripts])