- djm@cvs.openbsd.org 2009/09/01 14:43:17

     [ssh-agent.c]
     fix a race condition in ssh-agent that could result in a wedged or
     spinning agent: don't read off the end of the allocated fd_sets, and
     don't issue blocking read/write on agent sockets - just fall back to
     select() on retriable read/write errors. bz#1633 reported and tested
     by "noodle10000 AT googlemail.com"; ok dtucker@ markus@

1 files changed, 15 insertions, 21 deletions

diff --git a/ssh-agent.c b/ssh-agent.c
index f77dea3a6..df3a87d9a 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.161 2009/03/23 19:38:04 tobias Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.162 2009/09/01 14:43:17 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -919,11 +919,11 @@ after_select(fd_set *readset, fd_set *writeset)
         socklen_t slen;
         char buf[1024];
         int len, sock;
-        u_int i;
+        u_int i, orig_alloc;
         uid_t euid;
         gid_t egid;
 
-        for (i = 0; i < sockets_alloc; i++)
+        for (i = 0, orig_alloc = sockets_alloc; i < orig_alloc; i++)
                 switch (sockets[i].type) {
                 case AUTH_UNUSED:
                         break;
@@ -956,16 +956,13 @@ after_select(fd_set *readset, fd_set *writeset)
                 case AUTH_CONNECTION:
                         if (buffer_len(&sockets[i].output) > 0 &&
                             FD_ISSET(sockets[i].fd, writeset)) {
-                                do {
-                                        len = write(sockets[i].fd,
-                                            buffer_ptr(&sockets[i].output),
-                                            buffer_len(&sockets[i].output));
-                                        if (len == -1 && (errno == EAGAIN ||
-                                            errno == EINTR ||
-                                            errno == EWOULDBLOCK))
-                                                continue;
-                                        break;
-                                } while (1);
+                                len = write(sockets[i].fd,
+                                    buffer_ptr(&sockets[i].output),
+                                    buffer_len(&sockets[i].output));
+                                if (len == -1 && (errno == EAGAIN ||
+                                    errno == EWOULDBLOCK ||
+                                    errno == EINTR))
+                                        continue;
                                 if (len <= 0) {
                                         close_socket(&sockets[i]);
                                         break;
@@ -973,14 +970,11 @@ after_select(fd_set *readset, fd_set *writeset)
                                 buffer_consume(&sockets[i].output, len);
                         }
                         if (FD_ISSET(sockets[i].fd, readset)) {
-                                do {
-                                        len = read(sockets[i].fd, buf, sizeof(buf));
-                                        if (len == -1 && (errno == EAGAIN ||
-                                            errno == EINTR ||
-                                            errno == EWOULDBLOCK))
-                                                continue;
-                                        break;
-                                } while (1);
+                                len = read(sockets[i].fd, buf, sizeof(buf));
+                                if (len == -1 && (errno == EAGAIN ||
+                                    errno == EWOULDBLOCK ||
+                                    errno == EINTR))
+                                        continue;
                                 if (len <= 0) {
                                         close_socket(&sockets[i]);
                                         break;