From 10cdee3bc55274d7f6278cdd0c66edacb5b520a5 Mon Sep 17 00:00:00 2001 From: rofl0r Date: Tue, 15 Sep 2020 19:29:03 +0100 Subject: prepare transition to poll() usage of select() is inefficient (because a huge fd_set array has to be initialized on each call) and insecure (because an fd >= FD_SETSIZE will cause out-of-bounds accesses using the FD_*SET macros, and a system can be set up to allow more than that number of fds using ulimit). for the moment we prepared a poll-like wrapper that still runs select() to test for regressions, and so we have fallback code for systems without poll(). --- src/Makefile.am | 1 + src/child.c | 33 +++++++++++++++------------------ src/mypoll.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/mypoll.h | 22 ++++++++++++++++++++++ src/reqs.c | 44 +++++++++++++++++++------------------------- 5 files changed, 99 insertions(+), 43 deletions(-) create mode 100644 src/mypoll.c create mode 100644 src/mypoll.h diff --git a/src/Makefile.am b/src/Makefile.am index fd8a499..4973eb7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -51,6 +51,7 @@ tinyproxy_SOURCES = \ hsearch.c hsearch.h \ orderedmap.c orderedmap.h \ loop.c loop.h \ + mypoll.c mypoll.h \ connect-ports.c connect-ports.h EXTRA_tinyproxy_SOURCES = filter.c filter.h \ diff --git a/src/child.c b/src/child.c index 107475b..96864b1 100644 --- a/src/child.c +++ b/src/child.c @@ -34,6 +34,7 @@ #include "sblist.h" #include "loop.h" #include "conns.h" +#include "mypoll.h" #include static vector_t listen_fds; @@ -81,9 +82,10 @@ void child_main_loop (void) union sockaddr_union cliaddr_storage; struct sockaddr *cliaddr = (void*) &cliaddr_storage; socklen_t clilen = sizeof(cliaddr_storage); - fd_set rfds; + int nfds = vector_length(listen_fds); + pollfd_struct *fds = safecalloc(nfds, sizeof *fds); ssize_t i; - int ret, listenfd, maxfd, was_full = 0; + int ret, listenfd, was_full = 0; pthread_attr_t *attrp, attr; struct child *child; @@ -91,6 +93,12 @@ void child_main_loop (void) loop_records_init(); + for (i = 0; i < nfds; i++) { + int *fd = (int *) vector_getentry(listen_fds, i, NULL); + fds[i].fd = *fd; + fds[i].events |= MYPOLL_READ; + } + /* * We have to wait for connections on multiple fds, * so use select. @@ -111,7 +119,6 @@ void child_main_loop (void) was_full = 0; listenfd = -1; - maxfd = 0; /* Handle log rotation if it was requested */ if (received_sighup) { @@ -125,17 +132,8 @@ void child_main_loop (void) received_sighup = FALSE; } + ret = mypoll(fds, nfds, -1); - FD_ZERO(&rfds); - - for (i = 0; i < vector_length(listen_fds); i++) { - int *fd = (int *) vector_getentry(listen_fds, i, NULL); - - FD_SET(*fd, &rfds); - maxfd = max(maxfd, *fd); - } - - ret = select(maxfd + 1, &rfds, NULL, NULL, NULL); if (ret == -1) { if (errno == EINTR) { continue; @@ -149,15 +147,13 @@ void child_main_loop (void) continue; } - for (i = 0; i < vector_length(listen_fds); i++) { - int *fd = (int *) vector_getentry(listen_fds, i, NULL); - - if (FD_ISSET(*fd, &rfds)) { + for (i = 0; i < nfds; i++) { + if (fds[i].revents & MYPOLL_READ) { /* * only accept the connection on the first * fd that we find readable. - fair? */ - listenfd = *fd; + listenfd = fds[i].fd; break; } } @@ -220,6 +216,7 @@ oom: goto oom; } } + safefree(fds); } /* diff --git a/src/mypoll.c b/src/mypoll.c new file mode 100644 index 0000000..0e06805 --- /dev/null +++ b/src/mypoll.c @@ -0,0 +1,42 @@ +#include "mypoll.h" + +#define MYPOLL_READ (1<<1) +#define MYPOLL_WRITE (1<<2) + +int mypoll(pollfd_struct* fds, int nfds, int timeout) { + fd_set rset, wset, *r=0, *w=0; + int i, ret, maxfd=-1; + struct timeval tv = {0}, *t = 0; + + for(i=0; i maxfd) maxfd = fds[i].fd; + if(fds[i].events & MYPOLL_READ) FD_SET(fds[i].fd, r); + if(fds[i].events & MYPOLL_WRITE) FD_SET(fds[i].fd, w); + } + + if(timeout >= 0) t = &tv; + if(timeout > 0) tv.tv_sec = timeout; + + ret = select(maxfd+1, r, w, 0, t); + + switch(ret) { + case -1: + case 0: + return ret; + } + + for(i=0; i + +#ifdef HAVE_POLL_H +#include +typedef struct pollfd pollfd_struct; +#else +typedef struct mypollfd { + int fd; + short events; + short revents; +} pollfd_struct; +#endif + +#define MYPOLL_READ (1<<1) +#define MYPOLL_WRITE (1<<2) + +int mypoll(pollfd_struct* fds, int nfds, int timeout); + +#endif diff --git a/src/reqs.c b/src/reqs.c index 2b7649e..6bb456b 100644 --- a/src/reqs.c +++ b/src/reqs.c @@ -51,6 +51,7 @@ #include "conf.h" #include "basicauth.h" #include "loop.h" +#include "mypoll.h" /* * Maximum length of a HTTP line @@ -1141,29 +1142,24 @@ ERROR_EXIT: */ static void relay_connection (struct conn_s *connptr) { - fd_set rset, wset; - struct timeval tv; int ret; - int maxfd = max (connptr->client_fd, connptr->server_fd) + 1; ssize_t bytes_received; for (;;) { - tv.tv_sec = config->idletimeout; - tv.tv_usec = 0; - - FD_ZERO (&rset); - FD_ZERO (&wset); + pollfd_struct fds[2] = {0}; + fds[0].fd = connptr->client_fd; + fds[1].fd = connptr->server_fd; if (buffer_size (connptr->sbuffer) > 0) - FD_SET (connptr->client_fd, &wset); + fds[0].events |= MYPOLL_WRITE; if (buffer_size (connptr->cbuffer) > 0) - FD_SET (connptr->server_fd, &wset); + fds[1].events |= MYPOLL_WRITE; if (buffer_size (connptr->sbuffer) < MAXBUFFSIZE) - FD_SET (connptr->server_fd, &rset); + fds[1].events |= MYPOLL_READ; if (buffer_size (connptr->cbuffer) < MAXBUFFSIZE) - FD_SET (connptr->client_fd, &rset); + fds[0].events |= MYPOLL_READ; - ret = select (maxfd, &rset, &wset, NULL, &tv); + ret = mypoll(fds, 2, config->idletimeout); if (ret == 0) { log_message (LOG_INFO, @@ -1178,7 +1174,7 @@ static void relay_connection (struct conn_s *connptr) return; } - if (FD_ISSET (connptr->server_fd, &rset)) { + if (fds[1].revents & MYPOLL_READ) { bytes_received = read_buffer (connptr->server_fd, connptr->sbuffer); if (bytes_received < 0) @@ -1188,15 +1184,15 @@ static void relay_connection (struct conn_s *connptr) if (connptr->content_length.server == 0) break; } - if (FD_ISSET (connptr->client_fd, &rset) + if ((fds[0].revents & MYPOLL_READ) && read_buffer (connptr->client_fd, connptr->cbuffer) < 0) { break; } - if (FD_ISSET (connptr->server_fd, &wset) + if ((fds[1].revents & MYPOLL_WRITE) && write_buffer (connptr->server_fd, connptr->cbuffer) < 0) { break; } - if (FD_ISSET (connptr->client_fd, &wset) + if ((fds[0].revents & MYPOLL_WRITE) && write_buffer (connptr->client_fd, connptr->sbuffer) < 0) { break; } @@ -1435,14 +1431,12 @@ static int get_request_entity(struct conn_s *connptr) { int ret; - fd_set rset; - struct timeval tv; + pollfd_struct fds[1] = {0}; - FD_ZERO (&rset); - FD_SET (connptr->client_fd, &rset); - tv.tv_sec = config->idletimeout; - tv.tv_usec = 0; - ret = select (connptr->client_fd + 1, &rset, NULL, NULL, &tv); + fds[0].fd = connptr->client_fd; + fds[0].events |= MYPOLL_READ; + + ret = mypoll(fds, 1, config->idletimeout); if (ret == -1) { log_message (LOG_ERR, @@ -1450,7 +1444,7 @@ get_request_entity(struct conn_s *connptr) connptr->client_fd, strerror(errno)); } else if (ret == 0) { log_message (LOG_INFO, "no entity"); - } else if (ret == 1 && FD_ISSET (connptr->client_fd, &rset)) { + } else if (ret == 1 && (fds[0].revents & MYPOLL_READ)) { ssize_t nread; nread = read_buffer (connptr->client_fd, connptr->cbuffer); if (nread < 0) { -- cgit v1.2.3