From 224f0ca1acc136f8139d3fcdc94af9b3c7fd8b72 Mon Sep 17 00:00:00 2001 From: deuce <> Date: Fri, 1 May 2015 04:05:49 +0000 Subject: [PATCH] A deadlock could occur if you called conn_close() while the remote was sending a buttload of data. If the input queue filled up before the socket was closed, the input queue could be stuck waiting for the ring buffer to drain while conn_close() is stuck waiting for the input thread to stop. We now consume data from the input buffer while waiting for the input thread to stop. --- src/syncterm/conn_pty.c | 5 ++++- src/syncterm/conn_telnet.c | 6 +++++- src/syncterm/modem.c | 5 ++++- src/syncterm/rlogin.c | 6 +++++- src/syncterm/ssh.c | 6 +++++- src/syncterm/syncterm.c | 2 ++ 6 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/syncterm/conn_pty.c b/src/syncterm/conn_pty.c index 024713a2c3..232ab7fb16 100644 --- a/src/syncterm/conn_pty.c +++ b/src/syncterm/conn_pty.c @@ -502,6 +502,7 @@ int pty_connect(struct bbslist *bbs) int pty_close(void) { time_t start; + char garbage[1024]; conn_api.terminate=1; start=time(NULL); @@ -515,8 +516,10 @@ int pty_close(void) kill(child_pid, SIGKILL); waitpid(child_pid, &status, 0); - while(conn_api.input_thread_running || conn_api.output_thread_running) + while(conn_api.input_thread_running || conn_api.output_thread_running) { + conn_recv_upto(garbage, sizeof(garbage), 0); SLEEP(1); + } destroy_conn_buf(&conn_inbuf); destroy_conn_buf(&conn_outbuf); FREE_AND_NULL(conn_api.rd_buf); diff --git a/src/syncterm/conn_telnet.c b/src/syncterm/conn_telnet.c index 29e50fbda9..2490108add 100644 --- a/src/syncterm/conn_telnet.c +++ b/src/syncterm/conn_telnet.c @@ -170,10 +170,14 @@ int telnet_connect(struct bbslist *bbs) int telnet_close(void) { + char garbage[1024]; + conn_api.terminate=1; closesocket(telnet_sock); - while(conn_api.input_thread_running || conn_api.output_thread_running) + while(conn_api.input_thread_running || conn_api.output_thread_running) { + conn_recv_upto(garbage, sizeof(garbage), 0); SLEEP(1); + } destroy_conn_buf(&conn_inbuf); destroy_conn_buf(&conn_outbuf); FREE_AND_NULL(conn_api.rd_buf); diff --git a/src/syncterm/modem.c b/src/syncterm/modem.c index 22120ab349..3951856133 100644 --- a/src/syncterm/modem.c +++ b/src/syncterm/modem.c @@ -313,6 +313,7 @@ int serial_close(void) int modem_close(void) { time_t start; + char garbage[1024]; conn_api.terminate=1; @@ -333,8 +334,10 @@ int modem_close(void) } CLOSEIT: - while(conn_api.input_thread_running || conn_api.output_thread_running) + while(conn_api.input_thread_running || conn_api.output_thread_running) { + conn_recv_upto(garbage, sizeof(garbage), 0); SLEEP(1); + } comClose(com); destroy_conn_buf(&conn_inbuf); diff --git a/src/syncterm/rlogin.c b/src/syncterm/rlogin.c index 865478278b..5351e13237 100644 --- a/src/syncterm/rlogin.c +++ b/src/syncterm/rlogin.c @@ -174,10 +174,14 @@ int rlogin_connect(struct bbslist *bbs) int rlogin_close(void) { + char garbage[1024]; + conn_api.terminate=1; closesocket(sock); - while(conn_api.input_thread_running || conn_api.output_thread_running) + while(conn_api.input_thread_running || conn_api.output_thread_running) { + conn_recv_upto(garbage, sizeof(garbage), 0); SLEEP(1); + } 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 8ec4ebbaf2..e2be607e38 100644 --- a/src/syncterm/ssh.c +++ b/src/syncterm/ssh.c @@ -295,11 +295,15 @@ int ssh_connect(struct bbslist *bbs) int ssh_close(void) { + char garbage[1024]; + conn_api.terminate=1; ssh_active=FALSE; cl.SetAttribute(ssh_session, CRYPT_SESSINFO_ACTIVE, 0); - while(conn_api.input_thread_running || conn_api.output_thread_running) + while(conn_api.input_thread_running || conn_api.output_thread_running) { + conn_recv_upto(garbage, sizeof(garbage), 0); SLEEP(1); + } cl.DestroySession(ssh_session); closesocket(sock); sock=INVALID_SOCKET; diff --git a/src/syncterm/syncterm.c b/src/syncterm/syncterm.c index 0f1a36d835..b36432d27e 100644 --- a/src/syncterm/syncterm.c +++ b/src/syncterm/syncterm.c @@ -1612,6 +1612,8 @@ int main(int argc, char **argv) last_bbs=strdup(bbs->name); bbs=NULL; } + if (last_bbs) + free(last_bbs); // Save changed settings if(getscaling() > 0 && getscaling() != settings.scaling_factor) { char inipath[MAX_PATH+1]; -- GitLab