Hi,
I fell on this using openssh on cygwin. Though it may be a cygwin
related issue, I think it's may be a bug on the main openssh tree. Thus
my posting here. I'm CC'ing to the public list for information.
The part of code I'm refering to is :
/* XXX might close listen socket */
(void)dup2(fd, STDIN_FILENO);
(void)dup2(fd, STDOUT_FILENO);
(void)dup2(fd, STDERR_FILENO);
if (fd > 2)
close(fd);
I'm actually launching the ssh agent from a "run.exe" script
launched at
the start of my X server (Cygwin/X). Things used to work perfectly until
my last update. Unfortunately, I don't know which version I used before :-(
Something may have changed in the cygwin implementation.
What happens is that the agent is apparently launched without any opened
file descriptors (as far as I can see). So the auth socket gets the fd
0. We therefore fall on the "/* XXX might close listen socket */"
case...
I suggest a simple patch here :
if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
if (sock == 0) {
dup2(sock, fd+1) ;
close(sock) ;
sock = fd+1 ;
}
(void)dup2(fd, STDIN_FILENO);
(void)dup2(fd, STDOUT_FILENO);
(void)dup2(fd, STDERR_FILENO);
if (fd > 2)
close(fd);
}
It would be possible to dup2 the socket a second time after the
"close(fd)" so that it would use fd instead of fd+1, but I don't
really
see the point.
Also, it might be cleaner to do a :
if ((sock == STDIN_FILENO) || (sock == STDOUT_FILENO) || (sock ==
STDERR_FILENO))
Thanks for you feedback.
Cheers,
Fr?d?ric Olivi?.
Fr?d?ric Olivi? wrote:> Hi, > > I fell on this using openssh on cygwin. Though it may be a cygwin > related issue, I think it's may be a bug on the main openssh tree. Thus > my posting here. I'm CC'ing to the public list for information.This is already fixed in -current. Please try a snapshot release. -d
Please don't send HTML mail. On Wed, 4 Jan 2006, Fr?d?ric Olivi? wrote:> Hi, > > I tested and reviewed the last CVS release. > > The patch which fixed this problem is wrong (sorry) for many reasons : > > 1) Doing a this sanitize_fd() like it is at the beginning of the main() is > plain wrong. What happens in this specific case is that fd 0 is closed at exec > time, but fd 1 and 2 are opened. And we definitely need one of them so that > ssh-agent can send it's environment vars on stdout.Please try this diff: Index: misc.c ==================================================================RCS file: /cvs/src/usr.bin/ssh/misc.c,v retrieving revision 1.40 diff -u -p -r1.40 misc.c --- misc.c 2 Jan 2006 07:53:44 -0000 1.40 +++ misc.c 4 Jan 2006 21:45:34 -0000 @@ -601,18 +601,21 @@ tun_open(int tun, int mode) void sanitise_stdfd(void) { - int nullfd; + int nullfd, dupfd; - if ((nullfd = open(_PATH_DEVNULL, O_RDWR)) == -1) { + if ((nullfd = dupfd = open(_PATH_DEVNULL, O_RDWR)) == -1) { fprintf(stderr, "Couldn't open /dev/null: %s", strerror(errno)); exit(1); } - while (nullfd < 2) { - if (dup2(nullfd, nullfd + 1) == -1) { + while (dupfd < 2) { + /* Only clobber closed fds */ + if (fcntl(dupfd, F_GETFL, 0) >= 0) + continue; + if (dup2(nullfd, dupfd) == -1) { fprintf(stderr, "dup2: %s", strerror(errno)); exit(1); } - nullfd++; + dupfd++; } if (nullfd > 2) close(nullfd);