Skip to content

Commit

Permalink
prevent mutexes from being moved in memory
Browse files Browse the repository at this point in the history
Moving a pthread_mutex in memory (e.g. via realloc()) is undefined
behavior.
  • Loading branch information
sudden6 committed Oct 13, 2019
1 parent eda5fa4 commit a324c6e
Showing 1 changed file with 37 additions and 20 deletions.
57 changes: 37 additions & 20 deletions toxcore/net_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ typedef struct Crypto_Connection {

uint8_t maximum_speed_reached;

pthread_mutex_t mutex;
/* Must be a pointer, because the struct is moved in memory */
pthread_mutex_t* mutex;

dht_pk_cb *dht_pk_callback;
void *dht_pk_callback_object;
Expand Down Expand Up @@ -680,7 +681,7 @@ static int send_packet_to(Net_Crypto *c, int crypt_connection_id, const uint8_t

int direct_send_attempt = 0;

pthread_mutex_lock(&conn->mutex);
pthread_mutex_lock(conn->mutex);
IP_Port ip_port = return_ip_port_connection(c, crypt_connection_id);

// TODO(irungentoo): on bad networks, direct connections might not last indefinitely.
Expand All @@ -690,11 +691,11 @@ static int send_packet_to(Net_Crypto *c, int crypt_connection_id, const uint8_t

if (direct_connected) {
if ((uint32_t)sendpacket(dht_get_net(c->dht), ip_port, data, length) == length) {
pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);
return 0;
}

pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);
return -1;
}

Expand All @@ -710,18 +711,18 @@ static int send_packet_to(Net_Crypto *c, int crypt_connection_id, const uint8_t
}
}

pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);
pthread_mutex_lock(&c->tcp_mutex);
int ret = send_packet_tcp_connection(c->tcp_c, conn->connection_number_tcp, data, length);
pthread_mutex_unlock(&c->tcp_mutex);

pthread_mutex_lock(&conn->mutex);
pthread_mutex_lock(conn->mutex);

if (ret == 0) {
conn->last_tcp_sent = current_time_monotonic(c->mono_time);
}

pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);

if (ret == 0 || direct_send_attempt) {
return 0;
Expand Down Expand Up @@ -1073,19 +1074,19 @@ static int send_data_packet(Net_Crypto *c, int crypt_connection_id, const uint8_
return -1;
}

pthread_mutex_lock(&conn->mutex);
pthread_mutex_lock(conn->mutex);
VLA(uint8_t, packet, 1 + sizeof(uint16_t) + length + CRYPTO_MAC_SIZE);
packet[0] = NET_PACKET_CRYPTO_DATA;
memcpy(packet + 1, conn->sent_nonce + (CRYPTO_NONCE_SIZE - sizeof(uint16_t)), sizeof(uint16_t));
const int len = encrypt_data_symmetric(conn->shared_key, conn->sent_nonce, data, length, packet + 1 + sizeof(uint16_t));

if (len + 1 + sizeof(uint16_t) != SIZEOF_VLA(packet)) {
pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);
return -1;
}

increment_nonce(conn->sent_nonce);
pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);

return send_packet_to(c, crypt_connection_id, packet, SIZEOF_VLA(packet));
}
Expand Down Expand Up @@ -1172,9 +1173,9 @@ static int64_t send_lossless_packet(Net_Crypto *c, int crypt_connection_id, cons
dt.sent_time = 0;
dt.length = length;
memcpy(dt.data, data, length);
pthread_mutex_lock(&conn->mutex);
pthread_mutex_lock(conn->mutex);
int64_t packet_num = add_data_end_of_buffer(c->log, &conn->send_array, &dt);
pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);

if (packet_num == -1) {
return -1;
Expand Down Expand Up @@ -1578,9 +1579,9 @@ static int handle_data_packet_core(Net_Crypto *c, int crypt_connection_id, const
}

while (1) {
pthread_mutex_lock(&conn->mutex);
pthread_mutex_lock(conn->mutex);
int ret = read_data_beg_buffer(c->log, &conn->recv_array, &dt);
pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);

if (ret == -1) {
break;
Expand Down Expand Up @@ -1750,6 +1751,16 @@ static int create_crypto_connection(Net_Crypto *c)
{
for (uint32_t i = 0; i < c->crypto_connections_length; ++i) {
if (c->crypto_connections[i].status == CRYPTO_CONN_NO_CONNECTION) {
/* On destruction the connection struct is zeroed and the mutex memory freed,
* allocate here again */
c->crypto_connections[i].mutex = (pthread_mutex_t*) malloc(sizeof(pthread_mutex_t));
if(c->crypto_connections[i].mutex == nullptr) {
return -1;
}
if (pthread_mutex_init(c->crypto_connections[i].mutex, nullptr) != 0) {
pthread_mutex_unlock(&c->connections_mutex);
return -1;
}
return i;
}
}
Expand All @@ -1776,8 +1787,12 @@ static int create_crypto_connection(Net_Crypto *c)
c->crypto_connections[id].last_packets_left_rem = 0;
c->crypto_connections[id].packet_send_rate_requested = 0;
c->crypto_connections[id].last_packets_left_requested_rem = 0;
c->crypto_connections[id].mutex = (pthread_mutex_t*) malloc(sizeof(pthread_mutex_t));
if(c->crypto_connections[id].mutex == nullptr) {
return -1;
}

if (pthread_mutex_init(&c->crypto_connections[id].mutex, nullptr) != 0) {
if (pthread_mutex_init(c->crypto_connections[id].mutex, nullptr) != 0) {
pthread_mutex_unlock(&c->connections_mutex);
return -1;
}
Expand All @@ -1800,7 +1815,9 @@ static int wipe_crypto_connection(Net_Crypto *c, int crypt_connection_id)

uint32_t i;

pthread_mutex_destroy(&c->crypto_connections[crypt_connection_id].mutex);
pthread_mutex_destroy(c->crypto_connections[crypt_connection_id].mutex);
// free memory before destroying the pointer to prevent a leak
free(c->crypto_connections[crypt_connection_id].mutex);
crypto_memzero(&c->crypto_connections[crypt_connection_id], sizeof(Crypto_Connection));

/* check if we can resize the connections array */
Expand Down Expand Up @@ -2422,15 +2439,15 @@ static int udp_handle_packet(void *object, IP_Port source, const uint8_t *packet
return -1;
}

pthread_mutex_lock(&conn->mutex);
pthread_mutex_lock(conn->mutex);

if (net_family_is_ipv4(source.ip.family)) {
conn->direct_lastrecv_timev4 = mono_time_get(c->mono_time);
} else {
conn->direct_lastrecv_timev6 = mono_time_get(c->mono_time);
}

pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);
return 0;
}

Expand Down Expand Up @@ -2821,10 +2838,10 @@ int send_lossy_cryptpacket(Net_Crypto *c, int crypt_connection_id, const uint8_t
int ret = -1;

if (conn) {
pthread_mutex_lock(&conn->mutex);
pthread_mutex_lock(conn->mutex);
uint32_t buffer_start = conn->recv_array.buffer_start;
uint32_t buffer_end = conn->send_array.buffer_end;
pthread_mutex_unlock(&conn->mutex);
pthread_mutex_unlock(conn->mutex);
ret = send_data_packet_helper(c, crypt_connection_id, buffer_start, buffer_end, data, length);
}

Expand Down

0 comments on commit a324c6e

Please sign in to comment.