Bug 1467 - improper handling of EWOULDBLOCK on HP
Summary: improper handling of EWOULDBLOCK on HP
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 5.0p1
Hardware: All HP-UX
: P2 normal
Assignee: Damien Miller
URL:
Keywords:
Depends on:
Blocks: V_5_1
  Show dependency treegraph
 
Reported: 2008-05-22 02:27 AEST by sconeu
Modified: 2023-01-13 13:56 AEDT (History)
2 users (show)

See Also:


Attachments
test for EWOULDBLOCK everywhere we currently check for EAGAIN (11.67 KB, patch)
2008-05-22 03:36 AEST, Damien Miller
no flags Details | Diff
revised patch (12.14 KB, patch)
2008-07-04 22:32 AEST, Damien Miller
dtucker: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sconeu 2008-05-22 02:27:34 AEST
This issue was first noted on SFTP, but the cause was down in the spawned ssh process.

On some systems (e.g. HP NonStop), read or write to a nonblocking socket will fail with EWOULDBLOCK instead of EAGAIN.  The code in channels.c does not handle EAGAIN, causing the socket to be closed, but the parent process does not recieve notification, leading to a stall.

in channel_handle_[erw]fd, the calls to read/write should check for EWOULDBLOCK as well as EAGAIN:

e.g, in channel_handle_wfd:

                len = write(c->wfd, buf, dlen);

                if (len < 0 &&
                   (errno == EINTR || 
#ifdef EWOULDBLOCK
                    errno == EWOULDBLOCK ||
#endif
                    errno == EAGAIN))

This appears to be pervasive throughout the code, not just in channels.c.
Comment 1 Damien Miller 2008-05-22 03:36:37 AEST
Created attachment 1506 [details]
test for EWOULDBLOCK everywhere we currently check for EAGAIN

I guess we could do something like this patch.

Does Nonstop define EAGAIN? If so, what is the difference between EAGAIN and EWOULDBLOCK?
Comment 2 sconeu 2008-05-22 04:55:39 AEST
Yes, NonStop defines EAGAIN, but doesn't use it in the case of a nonblocking socket.   The proposed fix is essentially what i've done in our internal port.

From the NonStop man page for write(2) (read is similar):

          [EAGAIN]  One of these conditions exists:


                      o  An attempt was made to write to a file
                         descriptor that cannot accept data, and the
                         O_NONBLOCK flag is set.

                      o  A write to a pipe (FIFO file) of PIPE_BUF
                         bytes or less is requested, O_NONBLOCK is
                         set, and fewer than nbytes of free space are
                         available.

                      o  The O_NONBLOCK flag is set on this file, and
                         the process would be delayed in the write
                         operation.

          [EWOULDBLOCK]
                    The process attempted an operation on a socket for
                    which O_NONBLOCK is set, there is no space
                    available, and no error has occurred.


My understanding is that EWOULDBLOCK is an older errno condition and newer systems don't use it.  The NonStop sockets implementation is apparently based on the older code.
Comment 3 sconeu 2008-05-22 05:01:58 AEST
I'm relatively new here, and am not sure of the proper etiquette.

I realize I gave a bad summary line (the effects rather than the cause).  The summary line to this bug should probably be changed to something more descriptive (e.g. "No check for EWOULDBLOCK").
Comment 4 sconeu 2008-05-22 12:41:12 AEST
Thanks, djm.

As an additional comment on this bug, the code in atomicio.c handled EWOULDBLOCK properly.
Comment 5 Damien Miller 2008-07-04 22:32:46 AEST
Created attachment 1541 [details]
revised patch

updated to -current
Comment 6 Damien Miller 2008-07-04 23:11:10 AEST
patch applied - this will be in openssh-5.1. Thanks!
Comment 7 Damien Miller 2008-07-22 12:22:45 AEST
Mass update RESOLVED->CLOSED after release of openssh-5.1