Page MenuHomePhabricator

0001-ssh-poll-timeout-fix.patch

File Metadata

Author
cortana63
Created
Mon, Nov 19, 8:54 PM

0001-ssh-poll-timeout-fix.patch

From 6f573abbed5af37a9fbaf2437f8d6e9cb70ac7ff Mon Sep 17 00:00:00 2001
From: magliocca <magliocca@arlut.utexas.edu>
Date: Mon, 19 Nov 2018 13:08:04 -0600
Subject: [PATCH 1/2] ssh poll timeout fix
Signed-off-by: Sarah Magliocca <magliocca@arlut.utexas.edu>
---
src/poll.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/poll.c b/src/poll.c
index 0ee8db2..63953fa 100644
--- a/src/poll.c
+++ b/src/poll.c
@@ -93,7 +93,10 @@ void ssh_poll_cleanup(void) {
}
int ssh_poll(ssh_pollfd_t *fds, nfds_t nfds, int timeout) {
- return poll((struct pollfd *) fds, nfds, timeout);
+ int rc = poll((struct pollfd *) fds, nfds, timeout);
+ if (timeout > 0 && rc <= 0)
+ return -1;
+ return rc;
}
#else /* HAVE_POLL */
@@ -258,7 +261,10 @@ void ssh_poll_cleanup(void) {
}
int ssh_poll(ssh_pollfd_t *fds, nfds_t nfds, int timeout) {
- return (ssh_poll_emu)(fds, nfds, timeout);
+ int rc = (ssh_poll_emu)(fds, nfds, timeout);
+ if (timeout > 0 && rc <= 0)
+ return -1;
+ return rc;
}
#endif /* HAVE_POLL */
--
1.8.3.1

Event Timeline

asn added a subscriber: asn.Tue, Nov 20, 9:42 AM

Why is that needed? Could you please give more details?

Before the fix: On Windows, during a successful download, ssh_poll() returns > 0. When there is a disconnect that occurs in the middle of the download, ssh_poll() returns 0.

ssh_poll_ctx_dopoll() checks if the return is 0, if so, it returns SSH_AGAIN. Because it's returning SSH_AGAIN, the calling method assumes it should attempt to poll again.

Following the stack trace up, it looks like ssh_handle_packets_termination() is where the while loop occurs - while there isn't an error, call ssh_handle_packets().

ssh_handle_packets() calls ssh_poll_ctx_dopoll() which calls ssh_poll() again. This results in an infinite loop during a disconnect.

For my fix, I am assuming that if the user provided a timeout to ssh_poll, then that means they would not want ssh_poll() to be called endlessly if there was a disconnect. So if there is a timeout and if there was a disconnect, return -1 for an error.

asn added a comment.Tue, Nov 20, 4:06 PM

I think, this only should be done in the ssh_poll() which uses the poll emulation (second hunk of this patch)! Also you need to add what you explained here as a comment into that function before the if-clause. This is important information!

And please *always* use blocks with brackets for if-clauses, see CVE-2014-1266 ... ;-)

asn added a comment.Tue, Nov 20, 4:07 PM

Oh, and thanks for your contribution!

Thank you for your feedback Andreas. I have attached the updated patch.