diff --git a/Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch b/Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch new file mode 100644 index 0000000..a7f59c7 --- /dev/null +++ b/Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch @@ -0,0 +1,61 @@ +From c947c161c6aba71322429fd28a42880c96055de4 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Thu, 14 Sep 2017 11:24:39 -0500 +Subject: [PATCH] Fix error handling in gpm_send_buffer/gpm_recv_buffer + +Signed-off-by: Alexander Scheel +Reviewed-by: Robbie Harwood +Merges: #213 +[rharwood@redhat.com: commit message formatting, copyright update] +(cherry picked from commit f2530fc280dd84e6abc0f5475e261aa0d2ee2a21) +--- + proxy/src/client/gpm_common.c | 18 ++++++------------ + 1 file changed, 6 insertions(+), 12 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index c91c099..994bd80 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -1,4 +1,4 @@ +-/* Copyright (C) 2011 the GSS-PROXY contributors, see COPYING for license */ ++/* Copyright (C) 2011,2017 the GSS-PROXY contributors, see COPYING for license */ + + #include "gssapi_gpm.h" + #include +@@ -413,10 +413,7 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx, + ret = 0; + + done: +- if (ret) { +- /* on errors we can only close the fd and return */ +- gpm_close_socket(gpmctx); +- } ++ /* we only need to return as gpm_retry_socket closes the socket */ + return ret; + } + +@@ -486,9 +483,10 @@ static int gpm_recv_buffer(struct gpm_ctx *gpmctx, + + done: + if (ret) { +- /* on errors we can only close the fd and return */ +- gpm_close_socket(gpmctx); +- gpm_epoll_close(gpmctx); ++ /* on errors, free the buffer to prevent calling ++ * xdr_destroy(&xdr_reply_ctx); */ ++ free(*buffer); ++ *buffer = NULL; + } + return ret; + } +@@ -563,10 +561,6 @@ static int gpm_send_recv_loop(struct gpm_ctx *gpmctx, char *send_buffer, + /* Close and reopen socket before trying again */ + ret = gpm_retry_socket(gpmctx); + +- /* Free buffer and set it to NULL to prevent free(xdr_reply_ctx) */ +- free(*recv_buffer); +- *recv_buffer = NULL; +- + if (ret != 0) + return ret; + ret = ETIMEDOUT; diff --git a/Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch b/Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch new file mode 100644 index 0000000..8a4bb0a --- /dev/null +++ b/Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch @@ -0,0 +1,79 @@ +From 94861421d2ba7bd910d53c088d0d9065aaa05708 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Thu, 14 Sep 2017 11:16:42 -0500 +Subject: [PATCH] Fix handling of non-EPOLLIN/EPOLLOUT events + +Signed-off-by: Alexander Scheel +Reviewed-by: Robbie Harwood +Merges: #213 +(cherry picked from commit b8f5b2f75612a11753cf742ee0477b98df8e6b02) +--- + proxy/src/client/gpm_common.c | 49 ++++++++++++++++++++++++++++++------------- + 1 file changed, 35 insertions(+), 14 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index c3ef96e..c91c099 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -281,26 +281,47 @@ static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) { + gpm_epoll_close(gpmctx); + } else if (epoll_ret == 1 && events[0].data.fd == gpmctx->timerfd) { + /* Got an event which is only our timer */ +- ret = read(gpmctx->timerfd, &timer_read, sizeof(uint64_t)); +- if (ret == -1 && errno != EAGAIN && errno != EWOULDBLOCK) { +- /* In the case when reading from the timer failed, don't hide the +- * timer error behind ETIMEDOUT such that it isn't retried */ +- ret = errno; ++ if ((events[0].events & EPOLLIN) == 0) { ++ /* We got an event which was not EPOLLIN; assume this is an error, ++ * and exit with EBADF: epoll_wait said timerfd had an event, ++ * but that event is not an EPOLIN event. */ ++ ret = EBADF; + } else { +- /* If ret == 0, then we definitely timed out. Else, if ret == -1 +- * and errno == EAGAIN or errno == EWOULDBLOCK, we're in a weird +- * edge case where epoll thinks the timer can be read, but it +- * is blocking more; treat it like a TIMEOUT and retry, as +- * nothing around us would handle EAGAIN from timer and retry +- * it. */ +- ret = ETIMEDOUT; ++ ret = read(gpmctx->timerfd, &timer_read, sizeof(uint64_t)); ++ if (ret == -1 && errno != EAGAIN && errno != EWOULDBLOCK) { ++ /* In the case when reading from the timer failed, don't hide the ++ * timer error behind ETIMEDOUT such that it isn't retried */ ++ ret = errno; ++ } else { ++ /* If ret == 0, then we definitely timed out. Else, if ret == -1 ++ * and errno == EAGAIN or errno == EWOULDBLOCK, we're in a weird ++ * edge case where epoll thinks the timer can be read, but it ++ * is blocking more; treat it like a TIMEOUT and retry, as ++ * nothing around us would handle EAGAIN from timer and retry ++ * it. */ ++ ret = ETIMEDOUT; ++ } + } + gpm_epoll_close(gpmctx); + } else { + /* If ret == 2, then we ignore the timerfd; that way if the next + * operation cannot be performed immediately, we timeout and retry. +- * If ret == 1 and data.fd == gpmctx->fd, return 0. */ +- ret = 0; ++ * Always check the returned event of the socket fd. */ ++ int fd_index = 0; ++ if (epoll_ret == 2 && events[fd_index].data.fd != gpmctx->fd) { ++ fd_index = 1; ++ } ++ ++ if ((events[fd_index].events & event_flags) == 0) { ++ /* We cannot call EPOLLIN/EPOLLOUT at this time; assume that this ++ * is a fatal error; return with EBADFD to distinguish from ++ * EBADF in timer_fd case. */ ++ ret = EBADFD; ++ gpm_epoll_close(gpmctx); ++ } else { ++ /* We definintely got a EPOLLIN/EPOLLOUT event; return success. */ ++ ret = 0; ++ } + } + + epoll_ret = epoll_ctl(gpmctx->epollfd, EPOLL_CTL_DEL, gpmctx->fd, NULL); diff --git a/Simplify-setting-NONBLOCK-on-socket.patch b/Simplify-setting-NONBLOCK-on-socket.patch new file mode 100644 index 0000000..322ca40 --- /dev/null +++ b/Simplify-setting-NONBLOCK-on-socket.patch @@ -0,0 +1,53 @@ +From 2d60f4ab0c74115877df00d23836e7d970eda7c4 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Thu, 14 Sep 2017 10:57:12 -0500 +Subject: [PATCH] Simplify setting NONBLOCK on socket + +Signed-off-by: Alexander Scheel +Reviewed-by: Simo Sorce +Reviewed-by: Robbie Harwood +Merges: #213 +[rharwood@redhat.com: fixup commit message] +(cherry picked from commit ec808ee6a5e6746ed35acc865f253425701be352) +--- + proxy/src/client/gpm_common.c | 15 +-------------- + 1 file changed, 1 insertion(+), 14 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index 5e097ce..c3ef96e 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -80,7 +80,6 @@ static int gpm_open_socket(struct gpm_ctx *gpmctx) + struct sockaddr_un addr = {0}; + char name[PATH_MAX]; + int ret; +- unsigned flags; + int fd = -1; + + ret = get_pipe_name(name); +@@ -92,24 +91,12 @@ static int gpm_open_socket(struct gpm_ctx *gpmctx) + strncpy(addr.sun_path, name, sizeof(addr.sun_path)-1); + addr.sun_path[sizeof(addr.sun_path)-1] = '\0'; + +- fd = socket(AF_UNIX, SOCK_STREAM, 0); ++ fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); + if (fd == -1) { + ret = errno; + goto done; + } + +- ret = fcntl(fd, F_GETFD, &flags); +- if (ret != 0) { +- ret = errno; +- goto done; +- } +- +- ret = fcntl(fd, F_SETFD, flags | O_NONBLOCK); +- if (ret != 0) { +- ret = errno; +- goto done; +- } +- + ret = connect(fd, (struct sockaddr *)&addr, sizeof(addr)); + if (ret == -1) { + ret = errno; diff --git a/gssproxy.spec b/gssproxy.spec index 389f322..2973b9d 100644 --- a/gssproxy.spec +++ b/gssproxy.spec @@ -1,6 +1,6 @@ Name: gssproxy Version: 0.7.0 -Release: 15%{?dist} +Release: 16%{?dist} Summary: GSSAPI Proxy Group: System Environment/Libraries @@ -29,6 +29,9 @@ Patch11: client-Switch-to-non-blocking-sockets.patch Patch12: server-Add-detailed-request-logging.patch Patch13: Fix-potential-free-of-non-heap-address.patch Patch14: Prevent-uninitialized-read-in-error-path-of-XDR-cont.patch +Patch15: Simplify-setting-NONBLOCK-on-socket.patch +Patch16: Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch +Patch17: Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch ### Dependencies ### Requires: krb5-libs >= 1.12.0 @@ -121,6 +124,9 @@ rm -rf %{buildroot} %systemd_postun_with_restart gssproxy.service %changelog +* Fri Sep 15 2017 Robbie Harwood - 0.7.0-16 +- Backport updates to epoll logic + * Tue Sep 12 2017 Robbie Harwood - 0.7.0-15 - Backport two security fixes