Skip to content

Commit

Permalink
Update socketio.c
Browse files Browse the repository at this point in the history
This is related to the win32-OpenSSH issue #787
PowerShell/Win32-OpenSSH#787

As request, the PR has been moved here

There is a bug in the Microsoft POSIX compatibility layer used to
translate POSIX socket API to Windows socket API. The bug concerns the
emulated function accept() (socketio_accept() in the file socketio.c),
which may return an invalid socket and may lock the program in an
infinite loop.

A race is happening between the Windows kernel signaling a socket
issue to the POSIX compatibility layer. If a connection to the ssh
service is dropped before being fully handled by the POSIX
compatibility layer, that layer may reach a state where the Windows
kernel is aware of the dropped connection and update all the socket
states of the Windows socket API. However, the POSIX compatibility
layer is in the middle of the accept() function task and fails without
updating the local state of the emulated POSIX socket.

When the emulated accept() exits, the current socket state is not
updated and the emulated select() call would carry on detecting
activity (previously already detected) on the socket, triggering the
same exact error in the accept() call, running then in an infinite
loop.

This may not happen all the time: if the Windows kernel signals the
error before the compatibility POSIX layer has setup its accept()
state and returned successfully, other calls such as send() and recv()
will update the state of the socket and will handle the issue.

However, on a loaded machine where the synchronization between the
Windows socket API and the compatibility POSIX layer may be slower,
the emulated accept() call may finish before being notified by the
Windows kernel of a client disconnection. This may be triggered with
nmap, which makes two TCP connections in a row quickly: one to detect
an opened port, and a second to retrieve the ssh banner.

The fix proposed is to update the emulated accept() function to modify
the internal state of the compatibility POSIX layer as a regular POSIX
kernel would do. I personally had this issue and this patch fixed it.

Please note that we have identified other situations where it could
potentially happen (in particular the socketio_setsockopt() call). But
we haven't investigated this issue more deeply. Other issues such as
PowerShell#414 or #606 may be related.
  • Loading branch information
itnic authored Dec 14, 2017
1 parent 8906783 commit 3a0de1b
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions contrib/win32/win32compat/socketio.c
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,7 @@ socketio_accept(struct w32_io* pio, struct sockaddr* addr, int* addrlen)
if (pio->read_details.error) {
errno = errno_from_WSAError(pio->read_details.error);
debug3("accept - ERROR: async io completed with error: %d, io:%p", pio->read_details.error, pio);
pio->read_details.error = 0;
goto on_error;
}

Expand Down

0 comments on commit 3a0de1b

Please sign in to comment.