[patch] ssh_handle_packets_termination() ignores timeout=0

[g4-lisz@xxxxxxxxxxxx]

Hi there,

I found some logic in ssh_handle_packets_termination() which probably
should be a different:

int ssh_handle_packets_termination(ssh_session session,
                                   long timeout,
                                   ssh_termination_function fct,
                                   void *user)
{
    struct ssh_timestamp ts;
    long timeout_ms = SSH_TIMEOUT_INFINITE;
    long tm;
    int ret = SSH_OK;

    /* If a timeout has been provided, use it */
    if (timeout > 0) {
        timeout_ms = timeout;
    } else {
        if (ssh_is_blocking(session)) {
            if (timeout == SSH_TIMEOUT_USER || timeout == SSH_TIMEOUT_DEFAULT) {
                if (session->opts.timeout > 0 ||
                    session->opts.timeout_usec > 0) {
                    timeout_ms =
                        ssh_make_milliseconds(session->opts.timeout,
                                              session->opts.timeout_usec);
                }
            }
        } else {
            timeout_ms = SSH_TIMEOUT_NONBLOCKING;
        }
    }

When timout is set to 0, this should actually make the poll return
immediately. But with the logic above this is only granted when the
session is non-blocking. If it's blocking, 0 as timout parameter is
simply ignored and the default SSH_TIMEOUT_INFINITE is used instead.

IMHO it should always return immediately for 0. Its called from within
ssh_channel_poll_timeout(). No one expects here that the functions
blocks for timeout = 0.

For example in connector.c: ssh_connector_set_event() makes use of this:

        if (ssh_channel_poll_timeout(connector->in_channel, 0, 0) > 0){
            connector->in_available = 1;
        }

Under certain circumstances, this blockes for a long time, if there is
no traffic on the channel.

A simple fix is attached: if (timeout >= 0) { timeout_ms = timeout; }

Another fix could be that whenever ssh_channel_poll_timeout() should not
block, we simply use ssh_channel_poll() and set the session to non-blocking:

  blocking = ssh_is_blocking(session);
  ssh_set_blocking(session, 0);
  rc = ssh_channel_poll(...);
  ssh_set_blocking(session,blocking);

Then we needed a patch for ssh_connector_set_event() ...

Regards,
Till

From d8a676878b566c7ee81a3bffdbc50680ab89bdca Mon Sep 17 00:00:00 2001
From: Till Wimmer <g4-lisz@xxxxxxxxxxxx>
Date: Tue, 29 Jan 2019 13:03:35 +0100
Subject: [PATCH] ssh_handle_packets_termination() also respects timeout=0 on
 blocking sessions

Signed-off-by: Till Wimmer <g4-lisz@xxxxxxxxxxxx>
---
 src/session.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/session.c b/src/session.c
index af9201f..3b151bf 100644
--- a/src/session.c
+++ b/src/session.c
@@ -684,7 +684,7 @@ int ssh_handle_packets_termination(ssh_session session,
     int ret = SSH_OK;
 
     /* If a timeout has been provided, use it */
-    if (timeout > 0) {
+    if (timeout >= 0) {
         timeout_ms = timeout;
     } else {
         if (ssh_is_blocking(session)) {
-- 
2.7.4