Page MenuHomePhabricator

ssh_connector_fd_cb() does not check for POLLHUP for in_fd
Closed, ResolvedPublic

Description

I work on a project called Multipass in which we use libssh for various needs in managing Ubuntu virtual machines. On of those needs is to execute binaries in the virtual machine via ssh. However, found this bug and determined it to be a bug in libssh. The issue is that when checking for events on the in_fd, POLLHUP should also be checked in case an EOF is sent.

Here is a diff that fixes the issue:

diff --git a/src/connector.c b/src/connector.c
index 6f15ee28..de249b6e 100644
--- a/src/connector.c
+++ b/src/connector.c
@@ -370,7 +370,7 @@ static int ssh_connector_fd_cb(ssh_poll_handle p,
 
     if (revents & POLLERR) {
         ssh_connector_except(connector, fd);
-    } else if((revents & POLLIN) && fd == connector->in_fd) {
+    } else if(((revents & POLLIN) || (revents & POLLHUP)) && fd == connector->in_fd) {
         ssh_connector_fd_in_cb(connector);
     } else if((revents & POLLOUT) && fd == connector->out_fd) {
         ssh_connector_fd_out_cb(connector);

Event Timeline

townsend created this task.Jan 19 2018, 9:23 PM
townsend updated the task description. (Show Details)Jan 25 2018, 2:17 PM
townsend added a project: Restricted Project.Jan 31 2018, 9:20 PM
albaguirre added a comment.EditedFeb 4 2018, 8:34 PM

To add some more detail..

For the case where you do "echo test | ssh tee foo", roughly the following sequence occurs.

  1. client code calls ssh_event_dopoll
  2. poll returns, and its revents bits are set to POLLIN | POLLHUP, sometimes only POLLIN, depending if the pipe closed
  3. ssh_connector_fd_cb callback is invoked in ssh_poll_ctx_dopoll
    1. ssh_connector_fd_in_cb does a read. Read returns 5 bytes (e.g. "test\n")
  4. client code calls ssh_event_dopoll again
  5. poll returns, revents only has the bit POLLHUP set.
  6. ssh_connector_fd_cb callback is invoked in ssh_poll_ctx_dopoll
  7. ssh_connector_fd_cb does nothing as POLLIN is not set, only POLLHUP as the pipe has closed
    1. client then just keeps spinning calling ssh_event_dopoll

By also checking for POLLHUP in ssh_connector_fd_cb for in_fd, then ssh_connector_fd_in_cb can be invoked which will finally do the necessary read that indicates EOF.

As you can see here: http://www.greenend.org.uk/rjk/tech/poll.html, on Linux poll only returns POLLHUP when the pipe has closed and there's no more data to read.

simos added a subscriber: simos.Feb 5 2018, 3:51 PM