openssh unix dev - Jun 2001 - openssh 2.9p1: data loss when stdout sent to a pipe

We recently tried upgrading openssh from 2.5.2p2 to 2.9p1
and discovered that it no longer worked to feed the output from a remote
command into a pipe, unless the output was short and the pipe was very
fast at processing its input.

Example 1:    ssh remote_machine some_command | less

(where "some_command" generates a lot of output) now fails after
the first screenful, with a "Write failed flushing stdout buffer"
message.

Example 2:    ssh remote_machine 'yes | head -2000' | (sleep 3 ; wc -l)

With openssh 2.5.2p2 this correctly returns "2000".
With 2.9p1 we get

    Write failed flushing stdout buffer.
       1024

This is on both Linux (RedHat 7.0) and Irix (6.5.11).

Investigation reveals that this problem was caused by the addition of
lines into clientloop.c that force non-blocking mode onto the stdout
channel. Applying the following patch (to revert that portion of
clientloop.c to its state as of openssh2.5.2p2) fixes the problem:

--- openssh-2.9p1/clientloop.c.losesdata	Fri Apr 20 08:50:51 2001
+++ openssh-2.9p1/clientloop.c	Fri Jun 15 14:13:32 2001
@@ -787,12 +787,15 @@

 	if (!compat20) {
 		/* enable nonblocking unless tty */
+                /* REMOVED for now since it prevents ssh command output from
+                   being reliably sent to a pipe. - PHS 2001/06/15
 		if (!isatty(fileno(stdin)))
 			set_nonblock(fileno(stdin));
 		if (!isatty(fileno(stdout)))
 			set_nonblock(fileno(stdout));
 		if (!isatty(fileno(stderr)))
 			set_nonblock(fileno(stderr));
+                */
 		max_fd = MAX(max_fd, fileno(stdin));
 		max_fd = MAX(max_fd, fileno(stdout));
 		max_fd = MAX(max_fd, fileno(stderr));

Now, presumably those lines were added to the source code to address some
other problem (perhaps one of the hanging problems I see mentioned on the
list?) So simply removing them as per my patch above, while it works fine
on our site, probably isn't the optimal solution. But leaving them in
isn't a solution either, since it broke several of our scripts that fed
ssh output into a pipe for processing!

--------------------------------------------+-------------------------------
Philip Spencer  pspencer at fields.utoronto.ca | Director of Computing Services
Room 336        (416)-348-9710  ext3036     | The Fields Institute for
222 College St, Toronto ON M5T 3J1 Canada   | Research in Mathematical Sciences

On Fri, Jun 15, 2001 at 02:19:44PM -0400, Philip Spencer
wrote:
> --- openssh-2.9p1/clientloop.c.losesdata	Fri Apr 20 08:50:51 2001
> +++ openssh-2.9p1/clientloop.c	Fri Jun 15 14:13:32 2001
> @@ -787,12 +787,15 @@
> 
>  	if (!compat20) {
>  		/* enable nonblocking unless tty */
> +                /* REMOVED for now since it prevents ssh command output
from
> +                   being reliably sent to a pipe. - PHS 2001/06/15
>  		if (!isatty(fileno(stdin)))
>  			set_nonblock(fileno(stdin));
>  		if (!isatty(fileno(stdout)))
>  			set_nonblock(fileno(stdout));
>  		if (!isatty(fileno(stderr)))
>  			set_nonblock(fileno(stderr));
> +                */
>  		max_fd = MAX(max_fd, fileno(stdin));
>  		max_fd = MAX(max_fd, fileno(stdout));
>  		max_fd = MAX(max_fd, fileno(stderr));
> 
> Now, presumably those lines were added to the source code to address some
> other problem (perhaps one of the hanging problems I see mentioned on the
> list?) So simply removing them as per my patch above, while it works fine
> on our site, probably isn't the optimal solution.
we now do this (in -current):

        /* restore blocking io */
        if (!isatty(fileno(stdin)))
                unset_nonblock(fileno(stdin));
        if (!isatty(fileno(stdout)))
                unset_nonblock(fileno(stdout));
        if (!isatty(fileno(stderr)))
                unset_nonblock(fileno(stderr));

before flushing stdout/stderr.

-m

We're seeing the same thing here. Sadly, the flush code is... ummm... 
differently written. It assumes _any_ return from write <= 0 is an 
unrecoverable error and bombs. It doesn't check for EAGAIN or EINTR, much 
less EWOULDBLOCK.

-- 
Carson Gaspar - carson at taltos.org
Queen trapped in a butch body