Backport updates to epoll logic

This commit is contained in:
Robbie Harwood 2017-09-15 15:21:34 +00:00
parent bb8257ecb7
commit 3757f7f8e4
4 changed files with 200 additions and 1 deletions

View File

@ -0,0 +1,61 @@
From c947c161c6aba71322429fd28a42880c96055de4 Mon Sep 17 00:00:00 2001
From: Alexander Scheel <alexander.m.scheel@gmail.com>
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 <alexander.m.scheel@gmail.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
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 <sys/types.h>
@@ -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;

View File

@ -0,0 +1,79 @@
From 94861421d2ba7bd910d53c088d0d9065aaa05708 Mon Sep 17 00:00:00 2001
From: Alexander Scheel <alexander.m.scheel@gmail.com>
Date: Thu, 14 Sep 2017 11:16:42 -0500
Subject: [PATCH] Fix handling of non-EPOLLIN/EPOLLOUT events
Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
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);

View File

@ -0,0 +1,53 @@
From 2d60f4ab0c74115877df00d23836e7d970eda7c4 Mon Sep 17 00:00:00 2001
From: Alexander Scheel <alexander.m.scheel@gmail.com>
Date: Thu, 14 Sep 2017 10:57:12 -0500
Subject: [PATCH] Simplify setting NONBLOCK on socket
Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
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;

View File

@ -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 <rharwood@redhat.com> - 0.7.0-16
- Backport updates to epoll logic
* Tue Sep 12 2017 Robbie Harwood <rharwood@redhat.com> - 0.7.0-15
- Backport two security fixes