From 69f23482ff4fbbba76a1bb6909cbd7e9bf9c7a0c Mon Sep 17 00:00:00 2001
From: anonymouspage <123-anonymouspage@users.noreply.gitlab.synchro.net>
Date: Tue, 18 Oct 2022 18:10:17 -0700
Subject: [PATCH] Use shutdown() ahead of close() for rlogin/ssh
Using only close() on a socket has a few issues:
(1) poll is not required to wake on such events leading to hangs
if infinite timeouts are used.
This can be observed in macOS:
(a) syncterm -iC
(b) connect via telnet to some system
(c) use the menu to disconnect from the system
Note: socket must be idle to reproduce.
(2) If a blocked recv/send receives an error in response to a close
from another thread, the context of the socket has been lost: the
descriptor is free to be reallocated under the thread performing
I/O by the system. Any retry logic that may be written in response
to the error condition could lead to operations being performed
on a valid but incorrect resource.
(3) Processes that inherited the socket via fork() will still have
a valid descriptor to the socket. i.e., the socket does not
globally shutdown via a call to close(); however, shutdown()
acts on the connection by triggering a TCP FIN. This will lead
to the actual shutdown of the socket by the remote connection
as well.
Solution:
Use shutdown() ahead of close(). This will initiate a proper shutdown
sequence on the socket while leaving the descriptor open. poll() will
wake, and a subsequent read or write will yield the desired EOF/EPIPE.
---
src/syncterm/rlogin.c | 3 ++-
src/syncterm/ssh.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/syncterm/rlogin.c b/src/syncterm/rlogin.c
index e309b1eae8..68fac96517 100644
--- a/src/syncterm/rlogin.c
+++ b/src/syncterm/rlogin.c
@@ -212,11 +212,12 @@ int rlogin_close(void)
char garbage[1024];
conn_api.terminate=1;
- closesocket(rlogin_sock);
+ shutdown(rlogin_sock, SHUT_RDWR);
while(conn_api.input_thread_running == 1 || conn_api.output_thread_running == 1) {
conn_recv_upto(garbage, sizeof(garbage), 0);
SLEEP(1);
}
+ closesocket(rlogin_sock);
destroy_conn_buf(&conn_inbuf);
destroy_conn_buf(&conn_outbuf);
FREE_AND_NULL(conn_api.rd_buf);
diff --git a/src/syncterm/ssh.c b/src/syncterm/ssh.c
index 0b45edaac5..c9e46c390d 100644
--- a/src/syncterm/ssh.c
+++ b/src/syncterm/ssh.c
@@ -355,6 +355,7 @@ int ssh_close(void)
SLEEP(1);
}
cl.DestroySession(ssh_session);
+ shutdown(SHUT_RDWR, ssh_sock);
closesocket(ssh_sock);
ssh_sock=INVALID_SOCKET;
destroy_conn_buf(&conn_inbuf);
--
GitLab