- 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@
This commit is contained in:
Darren Tucker 2009-10-07 09:01:03 +11:00
parent 7bee06ab3b
commit 72473c6b09
2 changed files with 22 additions and 21 deletions

View File

@ -73,6 +73,13 @@
- djm@cvs.openbsd.org 2009/08/31 21:01:29 - djm@cvs.openbsd.org 2009/08/31 21:01:29
[sftp-server.8] [sftp-server.8]
document -e and -h; prodded by jmc@ 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 20091002
- (djm) [Makefile.in] Mention readconf.o in ssh-keysign's make deps. - (djm) [Makefile.in] Mention readconf.o in ssh-keysign's make deps.

View File

@ -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> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * 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; socklen_t slen;
char buf[1024]; char buf[1024];
int len, sock; int len, sock;
u_int i; u_int i, orig_alloc;
uid_t euid; uid_t euid;
gid_t egid; 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) { switch (sockets[i].type) {
case AUTH_UNUSED: case AUTH_UNUSED:
break; break;
@ -956,16 +956,13 @@ after_select(fd_set *readset, fd_set *writeset)
case AUTH_CONNECTION: case AUTH_CONNECTION:
if (buffer_len(&sockets[i].output) > 0 && if (buffer_len(&sockets[i].output) > 0 &&
FD_ISSET(sockets[i].fd, writeset)) { FD_ISSET(sockets[i].fd, writeset)) {
do { len = write(sockets[i].fd,
len = write(sockets[i].fd, buffer_ptr(&sockets[i].output),
buffer_ptr(&sockets[i].output), buffer_len(&sockets[i].output));
buffer_len(&sockets[i].output)); if (len == -1 && (errno == EAGAIN ||
if (len == -1 && (errno == EAGAIN || errno == EWOULDBLOCK ||
errno == EINTR || errno == EINTR))
errno == EWOULDBLOCK)) continue;
continue;
break;
} while (1);
if (len <= 0) { if (len <= 0) {
close_socket(&sockets[i]); close_socket(&sockets[i]);
break; break;
@ -973,14 +970,11 @@ after_select(fd_set *readset, fd_set *writeset)
buffer_consume(&sockets[i].output, len); buffer_consume(&sockets[i].output, len);
} }
if (FD_ISSET(sockets[i].fd, readset)) { if (FD_ISSET(sockets[i].fd, readset)) {
do { len = read(sockets[i].fd, buf, sizeof(buf));
len = read(sockets[i].fd, buf, sizeof(buf)); if (len == -1 && (errno == EAGAIN ||
if (len == -1 && (errno == EAGAIN || errno == EWOULDBLOCK ||
errno == EINTR || errno == EINTR))
errno == EWOULDBLOCK)) continue;
continue;
break;
} while (1);
if (len <= 0) { if (len <= 0) {
close_socket(&sockets[i]); close_socket(&sockets[i]);
break; break;