From d270cf550abd168cfa7a96a8aceb78622bdfc92f Mon Sep 17 00:00:00 2001 From: irungentoo Date: Thu, 31 Jul 2014 12:46:36 -0400 Subject: [PATCH] Fixed possible threading issues. send_lossy_cryptpacket() can get called from another thread meaning the connection can be killed while the packet is sending. --- toxcore/net_crypto.c | 54 ++++++++++++++++++++++++++++++++++---------- toxcore/net_crypto.h | 5 +++- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/toxcore/net_crypto.c b/toxcore/net_crypto.c index 7607015e..57ee20b8 100644 --- a/toxcore/net_crypto.c +++ b/toxcore/net_crypto.c @@ -2443,7 +2443,7 @@ int64_t write_cryptpacket(const Net_Crypto *c, int crypt_connection_id, const ui * * Sends a lossy cryptopacket. (first byte must in the PACKET_ID_LOSSY_RANGE_*) */ -int send_lossy_cryptpacket(const Net_Crypto *c, int crypt_connection_id, const uint8_t *data, uint32_t length) +int send_lossy_cryptpacket(Net_Crypto *c, int crypt_connection_id, const uint8_t *data, uint32_t length) { if (length == 0 || length > MAX_CRYPTO_DATA_SIZE) return -1; @@ -2454,13 +2454,24 @@ int send_lossy_cryptpacket(const Net_Crypto *c, int crypt_connection_id, const u if (data[0] >= (PACKET_ID_LOSSY_RANGE_START + PACKET_ID_LOSSY_RANGE_SIZE)) return -1; + pthread_mutex_lock(&c->connections_mutex); + ++c->connection_use_counter; + pthread_mutex_unlock(&c->connections_mutex); + Crypto_Connection *conn = get_crypto_connection(c, crypt_connection_id); - if (conn == 0) - return -1; + int ret = -1; - return send_data_packet_helper(c, crypt_connection_id, conn->recv_array.buffer_start, conn->send_array.buffer_end, data, - length); + if (conn) { + ret = send_data_packet_helper(c, crypt_connection_id, conn->recv_array.buffer_start, conn->send_array.buffer_end, data, + length); + } + + pthread_mutex_lock(&c->connections_mutex); + --c->connection_use_counter; + pthread_mutex_unlock(&c->connections_mutex); + + return ret; } /* Kill a crypto connection. @@ -2470,17 +2481,32 @@ int send_lossy_cryptpacket(const Net_Crypto *c, int crypt_connection_id, const u */ int crypto_kill(Net_Crypto *c, int crypt_connection_id) { + while (1) { /* TODO: is this really the best way to do this? */ + pthread_mutex_lock(&c->connections_mutex); + + if (!c->connection_use_counter) { + break; + } + + pthread_mutex_unlock(&c->connections_mutex); + } + Crypto_Connection *conn = get_crypto_connection(c, crypt_connection_id); - if (conn == 0) - return -1; + int ret = -1; - if (conn->status == CRYPTO_CONN_ESTABLISHED) - send_kill_packet(c, crypt_connection_id); + if (conn) { + if (conn->status == CRYPTO_CONN_ESTABLISHED) + send_kill_packet(c, crypt_connection_id); - disconnect_peer_tcp(c, crypt_connection_id); - bs_list_remove(&c->ip_port_list, &conn->ip_port, crypt_connection_id); - return wipe_crypto_connection(c, crypt_connection_id); + disconnect_peer_tcp(c, crypt_connection_id); + bs_list_remove(&c->ip_port_list, &conn->ip_port, crypt_connection_id); + ret = wipe_crypto_connection(c, crypt_connection_id); + } + + pthread_mutex_unlock(&c->connections_mutex); + + return ret; } /* return one of CRYPTO_CONN_* values indicating the state of the connection. @@ -2553,6 +2579,8 @@ Net_Crypto *new_net_crypto(DHT *dht) networking_registerhandler(dht->net, NET_PACKET_CRYPTO_DATA, &udp_handle_packet, temp); bs_list_init(&temp->ip_port_list, sizeof(IP_Port), 8); + + pthread_mutex_init(&temp->connections_mutex, NULL); return temp; } @@ -2626,6 +2654,8 @@ void kill_net_crypto(Net_Crypto *c) kill_TCP_connection(c->tcp_connections[i]); } + pthread_mutex_destroy(&c->connections_mutex); + bs_list_free(&c->ip_port_list); networking_registerhandler(c->dht->net, NET_PACKET_COOKIE_REQUEST, NULL, NULL); networking_registerhandler(c->dht->net, NET_PACKET_COOKIE_RESPONSE, NULL, NULL); diff --git a/toxcore/net_crypto.h b/toxcore/net_crypto.h index 2bc9a716..07e9ef6a 100644 --- a/toxcore/net_crypto.h +++ b/toxcore/net_crypto.h @@ -179,6 +179,9 @@ typedef struct { TCP_Client_Connection *tcp_connections_new[MAX_TCP_CONNECTIONS]; TCP_Client_Connection *tcp_connections[MAX_TCP_CONNECTIONS]; + pthread_mutex_t connections_mutex; + unsigned int connection_use_counter; + uint32_t crypto_connections_length; /* Length of connections array. */ /* Our public and secret keys. */ @@ -302,7 +305,7 @@ int64_t write_cryptpacket(const Net_Crypto *c, int crypt_connection_id, const ui * * Sends a lossy cryptopacket. (first byte must in the PACKET_ID_LOSSY_RANGE_*) */ -int send_lossy_cryptpacket(const Net_Crypto *c, int crypt_connection_id, const uint8_t *data, uint32_t length); +int send_lossy_cryptpacket(Net_Crypto *c, int crypt_connection_id, const uint8_t *data, uint32_t length); /* Add a tcp relay, associating it to a crypt_connection_id. *