From 72473c6b09b61b5ea2a7bda7728062b89a46b394 Mon Sep 17 00:00:00 2001 From: Darren Tucker Date: Wed, 7 Oct 2009 09:01:03 +1100 Subject: [PATCH] - 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@ --- ChangeLog | 7 +++++++ ssh-agent.c | 36 +++++++++++++++--------------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5fbca0934..e961d1a2f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -73,6 +73,13 @@ - djm@cvs.openbsd.org 2009/08/31 21:01:29 [sftp-server.8] document -e and -h; prodded by jmc@ + - 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@ 20091002 - (djm) [Makefile.in] Mention readconf.o in ssh-keysign's make deps. 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 * Copyright (c) 1995 Tatu Ylonen , 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;