Skip to content
Snippets Groups Projects
Commit bde52d43 authored by Deucе's avatar Deucе :ok_hand_tone4:
Browse files

Split SFTP client cleanup into to functions.

sftp_finish() which ends the communication, disables further
commands, and can explicitly be called more than once, and
sftp_end() which frees the state.

This prevents a race around sftp_finish() calls and sftpc_*() calls
which is likely responsibe for various weirdness in SSH connections,
and should fix ticket 135.
parent 69322766
No related branches found
No related tags found
No related merge requests found
Pipeline #6091 passed
......@@ -250,6 +250,7 @@ bool sftpc_read(sftpc_state_t state, sftp_filehandle_t handle, uint64_t offset,
bool sftpc_write(sftpc_state_t state, sftp_filehandle_t handle, uint64_t offset, sftp_str_t data);
bool sftpc_reclaim(sftpc_state_t state);
uint32_t sftpc_get_err(sftpc_state_t state);
void sftpc_end(sftpc_state_t state);
/* sftp_attr.c */
sftp_file_attr_t sftp_fattr_alloc(void);
......
......@@ -147,6 +147,10 @@ sftpc_finish(sftpc_state_t state)
if (state == NULL)
return;
pthread_mutex_lock(&state->mtx);
if (state->terminating == true) {
pthread_mutex_unlock(&state->mtx);
return;
}
state->terminating = true;
pthread_mutex_unlock(&state->mtx);
// TODO: Close all open handles
......@@ -166,6 +170,17 @@ sftpc_finish(sftpc_state_t state)
sftp_free_rx_pkt(state->rxp);
sftp_free_tx_pkt(state->txp);
pthread_mutex_unlock(&state->mtx);
}
void
sftpc_end(sftpc_state_t state)
{
assert(state);
if (state == NULL)
return;
pthread_mutex_lock(&state->mtx);
assert(state->terminating);
pthread_mutex_unlock(&state->mtx);
pthread_mutex_destroy(&state->mtx);
free(state);
}
......
......@@ -101,7 +101,6 @@ cryptlib_error_message(int status, const char *msg)
static void
close_sftp_channel(int chan)
{
sftpc_state_t oldstate;
pthread_mutex_lock(&ssh_mutex);
if (chan != -1) {
FlushData(ssh_session);
......@@ -118,12 +117,8 @@ close_sftp_channel(int chan)
}
sftp_channel = -1;
}
oldstate = sftp_state;
sftp_state = NULL;
pthread_mutex_unlock(&ssh_mutex);
if (oldstate != NULL) {
sftpc_finish(oldstate);
}
sftpc_finish(sftp_state);
}
static void
......@@ -168,12 +163,13 @@ ssh_input_thread(void *args)
size_t buffered;
size_t buffer;
int chan;
sftpc_state_t oldstate = NULL;
bool both_gone = false;
bool sftp_do_finish;
SetThreadName("SSH Input");
conn_api.input_thread_running = 1;
while (ssh_active && !conn_api.terminate) {
sftp_do_finish = false;
pthread_mutex_lock(&ssh_mutex);
if (FlushData(ssh_session) == CRYPT_ERROR_COMPLETE) {
pthread_mutex_unlock(&ssh_mutex);
......@@ -185,8 +181,7 @@ ssh_input_thread(void *args)
}
if (sftp_channel != -1) {
if (!check_channel_open(&sftp_channel)) {
oldstate = sftp_state;
sftp_state = NULL;
sftp_do_finish = true;
sftp_channel = -1;
}
}
......@@ -195,9 +190,9 @@ ssh_input_thread(void *args)
pthread_mutex_unlock(&ssh_mutex);
if (both_gone)
break;
if (oldstate != NULL) {
sftpc_finish(oldstate);
oldstate = NULL;
if (sftp_do_finish) {
sftpc_finish(sftp_state);
sftp_do_finish = false;
}
if (!socket_readable(ssh_sock, 100))
continue;
......@@ -215,12 +210,8 @@ ssh_input_thread(void *args)
}
if (sftp_channel != -1) {
if (!check_channel_open(&sftp_channel)) {
sftpc_state_t oldstate = sftp_state;
sftp_state = NULL;
sftp_channel = -1;
pthread_mutex_unlock(&ssh_mutex);
sftpc_finish(oldstate);
oldstate = NULL;
sftpc_finish(sftp_state);
pthread_mutex_lock(&ssh_mutex);
}
}
......@@ -307,10 +298,6 @@ ssh_input_thread(void *args)
FlushData(ssh_session);
pthread_mutex_unlock(&ssh_mutex);
}
if (oldstate != NULL) {
sftpc_finish(oldstate);
oldstate = NULL;
}
conn_api.input_thread_running = 2;
}
......@@ -449,7 +436,7 @@ key_not_present(sftp_filehandle_t f, const char *priv)
sftp_str_t r = NULL;
bool skipread = false;
do {
while (ssh_active) {
if (skipread) {
old_bufpos = 0;
skipread = false;
......@@ -495,7 +482,8 @@ key_not_present(sftp_filehandle_t f, const char *priv)
memmove(buf, &buf[eol], bufpos - eol);
bufpos = bufpos - eol;
}
} while(1);
}
return false;
}
static void
......@@ -520,6 +508,8 @@ add_public_key(void *vpriv)
}
}
pthread_mutex_unlock(&ssh_mutex);
if (!ssh_active)
break;
SLEEP(10);
};
if (!active) {
......@@ -606,7 +596,7 @@ add_public_key(void *vpriv)
* To avoid that, we'll sleep for a second to allow
* the remote to close the channel if it wants to.
*/
for (unsigned sleep_count = 0; sleep_count < 100 && conn_api.terminate == 0; sleep_count++) {
for (unsigned sleep_count = 0; sleep_count < 100 && conn_api.terminate == 0 && ssh_active; sleep_count++) {
SLEEP(10);
}
pthread_mutex_lock(&ssh_tx_mutex);
......@@ -646,12 +636,7 @@ add_public_key(void *vpriv)
}
sftpc_close(sftp_state, &f);
}
pthread_mutex_lock(&ssh_mutex);
sftpc_state_t oldstate = sftp_state;
sftp_state = NULL;
pthread_mutex_unlock(&ssh_mutex);
if (oldstate)
sftpc_finish(oldstate);
sftpc_finish(sftp_state);
}
close_sftp_channel(new_sftp_channel);
}
......@@ -1022,16 +1007,21 @@ ssh_close(void)
cryptSetAttribute(ssh_session, CRYPT_OPTION_NET_READTIMEOUT, 1);
cryptSetAttribute(ssh_session, CRYPT_OPTION_NET_WRITETIMEOUT, 1);
conn_api.terminate = 1;
pthread_mutex_lock(&ssh_mutex);
int sc = sftp_channel;
pthread_mutex_unlock(&ssh_mutex);
close_sftp_channel(sc);
close_ssh_channel();
ssh_active = false;
if (sftp_state)
sftpc_finish(sftp_state);
while (conn_api.input_thread_running == 1 || conn_api.output_thread_running == 1 || pubkey_thread_running) {
conn_recv_upto(garbage, sizeof(garbage), 0);
SLEEP(1);
}
pthread_mutex_lock(&ssh_mutex);
int sc = sftp_channel;
pthread_mutex_unlock(&ssh_mutex);
if (sc != -1)
close_sftp_channel(sc);
if (sftp_state)
sftpc_end(sftp_state);
close_ssh_channel();
cryptDestroySession(ssh_session);
closesocket(ssh_sock);
ssh_sock = INVALID_SOCKET;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment