From 620108ea7770608de72dcbea4ca73d6fb99c1109 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Sep 2018 14:34:49 -0400 Subject: [PATCH 1/7] Assert that we aren't returning a pointer to a local variable. GCC got confused here with LTO enabled. Fixes part of #27772. --- src/feature/nodelist/routerparse.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/feature/nodelist/routerparse.c b/src/feature/nodelist/routerparse.c index b76b2974fa..ae57fbe499 100644 --- a/src/feature/nodelist/routerparse.c +++ b/src/feature/nodelist/routerparse.c @@ -4436,7 +4436,9 @@ router_parse_addr_policy(directory_token_t *tok, unsigned fmt_flags) return NULL; } - return addr_policy_get_canonical_entry(&newe); + addr_policy_t *result = addr_policy_get_canonical_entry(&newe); + tor_assert(result != &newe); // We aren't returning the one from the stack. + return result; } /** Parse an exit policy line of the format "accept[6]/reject[6] private:...". From 7ace8d5a61f75fb77e3619deed417edd5610a4f1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Sep 2018 14:43:57 -0400 Subject: [PATCH 2/7] Assert that some trunnel _new() functions return non-NULL The trunnel functions are written under the assumption that their allocators can fail, so GCC LTO thinks they might return NULL. In point of fact, they're using tor_malloc() and friends, which can't fail, but GCC won't necessarily figure that out. Fixes part of #27772. --- src/core/proto/proto_socks.c | 3 +++ src/feature/nodelist/torcert.c | 1 + src/lib/crypt_ops/crypto_pwbox.c | 1 + 3 files changed, 5 insertions(+) diff --git a/src/core/proto/proto_socks.c b/src/core/proto/proto_socks.c index ccf96f7814..e2f233ad05 100644 --- a/src/core/proto/proto_socks.c +++ b/src/core/proto/proto_socks.c @@ -353,6 +353,7 @@ process_socks5_methods_request(socks_request_t *req, int have_user_pass, { socks_result_t res = SOCKS_RESULT_DONE; socks5_server_method_t *trunnel_resp = socks5_server_method_new(); + tor_assert(trunnel_resp); socks5_server_method_set_version(trunnel_resp, SOCKS_VER_5); @@ -478,6 +479,7 @@ process_socks5_userpass_auth(socks_request_t *req) socks_result_t res = SOCKS_RESULT_DONE; socks5_server_userpass_auth_t *trunnel_resp = socks5_server_userpass_auth_new(); + tor_assert(trunnel_resp); if (req->socks_version != SOCKS_VER_5) { res = SOCKS_RESULT_INVALID; @@ -869,6 +871,7 @@ socks_request_set_socks5_error(socks_request_t *req, socks5_reply_status_t reason) { socks5_server_reply_t *trunnel_resp = socks5_server_reply_new(); + tor_assert(trunnel_resp); socks5_server_reply_set_version(trunnel_resp, SOCKS_VER_5); socks5_server_reply_set_reply(trunnel_resp, reason); diff --git a/src/feature/nodelist/torcert.c b/src/feature/nodelist/torcert.c index fe67e56403..f31e8ed77d 100644 --- a/src/feature/nodelist/torcert.c +++ b/src/feature/nodelist/torcert.c @@ -51,6 +51,7 @@ tor_cert_sign_impl(const ed25519_keypair_t *signing_key, tor_cert_t *torcert = NULL; ed25519_cert_t *cert = ed25519_cert_new(); + tor_assert(cert); // Trunnel's new functions can return NULL. cert->cert_type = cert_type; cert->exp_field = (uint32_t) CEIL_DIV(now + lifetime, 3600); cert->cert_key_type = signed_key_type; diff --git a/src/lib/crypt_ops/crypto_pwbox.c b/src/lib/crypt_ops/crypto_pwbox.c index 2377f216a0..91536e891b 100644 --- a/src/lib/crypt_ops/crypto_pwbox.c +++ b/src/lib/crypt_ops/crypto_pwbox.c @@ -61,6 +61,7 @@ crypto_pwbox(uint8_t **out, size_t *outlen_out, int rv; enc = pwbox_encoded_new(); + tor_assert(enc); pwbox_encoded_setlen_skey_header(enc, S2K_MAXLEN); From 965549aa07d94a9f9e510cdb7a215bf9a3ed7bb8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Sep 2018 15:06:36 -0400 Subject: [PATCH 3/7] Use assertions so GCC LTO doesn't worry about TLS channel conversion Part of #27772 --- src/core/mainloop/connection.c | 11 ++++++----- src/core/or/channel.c | 2 ++ src/core/or/channelpadding.c | 2 ++ src/core/or/scheduler_kist.c | 4 ++++ 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 0c3abc8442..7e2c4ce406 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -656,14 +656,15 @@ connection_free_minimal(connection_t *conn) tor_free(or_conn->nickname); if (or_conn->chan) { /* Owww, this shouldn't happen, but... */ + channel_t *base_chan = TLS_CHAN_TO_BASE(or_conn->chan); + tor_assert(base_chan); log_info(LD_CHANNEL, "Freeing orconn at %p, saw channel %p with ID " "%"PRIu64 " left un-NULLed", - or_conn, TLS_CHAN_TO_BASE(or_conn->chan), - ( - TLS_CHAN_TO_BASE(or_conn->chan)->global_identifier)); - if (!CHANNEL_FINISHED(TLS_CHAN_TO_BASE(or_conn->chan))) { - channel_close_for_error(TLS_CHAN_TO_BASE(or_conn->chan)); + or_conn, base_chan, + base_chan->global_identifier); + if (!CHANNEL_FINISHED(base_chan)) { + channel_close_for_error(base_chan); } or_conn->chan->conn = NULL; diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 0c204ddfb6..79c52235dc 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -3423,6 +3423,8 @@ channel_rsa_id_group_set_badness(struct channel_list_s *lst, int force) /* it would be more efficient to do a slice, but this case is rare */ smartlist_t *or_conns = smartlist_new(); SMARTLIST_FOREACH_BEGIN(channels, channel_t *, channel) { + tor_assert(channel); // Suppresses some compiler warnings. + if (!common_ed25519_identity) common_ed25519_identity = &channel->ed25519_identity; diff --git a/src/core/or/channelpadding.c b/src/core/or/channelpadding.c index 7c3a77f62c..d3d6890c0b 100644 --- a/src/core/or/channelpadding.c +++ b/src/core/or/channelpadding.c @@ -296,6 +296,7 @@ channelpadding_send_disable_command(channel_t *chan) channelpadding_negotiate_t disable; cell_t cell; + tor_assert(chan); tor_assert(BASE_CHAN_TO_TLS(chan)->conn->link_proto >= MIN_LINK_PROTO_FOR_CHANNEL_PADDING); @@ -328,6 +329,7 @@ channelpadding_send_enable_command(channel_t *chan, uint16_t low_timeout, channelpadding_negotiate_t enable; cell_t cell; + tor_assert(chan); tor_assert(BASE_CHAN_TO_TLS(chan)->conn->link_proto >= MIN_LINK_PROTO_FOR_CHANNEL_PADDING); diff --git a/src/core/or/scheduler_kist.c b/src/core/or/scheduler_kist.c index 41c346ac71..f112ea6357 100644 --- a/src/core/or/scheduler_kist.c +++ b/src/core/or/scheduler_kist.c @@ -116,6 +116,7 @@ static unsigned int kist_lite_mode = 1; static inline size_t channel_outbuf_length(channel_t *chan) { + tor_assert(chan); /* In theory, this can not happen because we can not scheduler a channel * without a connection that has its outbuf initialized. Just in case, bug * on this so we can understand a bit more why it happened. */ @@ -194,6 +195,8 @@ update_socket_info_impl, (socket_table_ent_t *ent)) { #ifdef HAVE_KIST_SUPPORT int64_t tcp_space, extra_space; + tor_assert(ent); + tor_assert(ent->chan); const tor_socket_t sock = TO_CONN(BASE_CHAN_TO_TLS((channel_t *) ent->chan)->conn)->s; struct tcp_info tcp; @@ -451,6 +454,7 @@ MOCK_IMPL(int, channel_should_write_to_kernel, * kernel */ MOCK_IMPL(void, channel_write_to_kernel, (channel_t *chan)) { + tor_assert(chan); log_debug(LD_SCHED, "Writing %lu bytes to kernel for chan %" PRIu64, (unsigned long)channel_outbuf_length(chan), chan->global_identifier); From 370d9922a4e3cb24a5ff093cadc3098771cab957 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Sep 2018 15:07:02 -0400 Subject: [PATCH 4/7] Use the correct function signatures in test_relaycell.c This is now officially an antipattern: please let's never copy a function declaration in two places again. That's what headers are for. --- src/core/or/connection_edge.c | 5 --- src/core/or/connection_edge.h | 6 ++++ src/test/test_relaycell.c | 63 +++++++++++++++-------------------- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index 35e68485b8..ab08e99b1e 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -155,11 +155,6 @@ static int connection_ap_process_natd(entry_connection_t *conn); static int connection_exit_connect_dir(edge_connection_t *exitconn); static int consider_plaintext_ports(entry_connection_t *conn, uint16_t port); static int connection_ap_supports_optimistic_data(const entry_connection_t *); -STATIC void connection_half_edge_add(const edge_connection_t *conn, - origin_circuit_t *circ); -STATIC half_edge_t *connection_half_edge_find_stream_id( - const smartlist_t *half_conns, - streamid_t stream_id); /** Convert a connection_t* to an edge_connection_t*; assert if the cast is * invalid. */ diff --git a/src/core/or/connection_edge.h b/src/core/or/connection_edge.h index 1348dd49f9..25e04f8416 100644 --- a/src/core/or/connection_edge.h +++ b/src/core/or/connection_edge.h @@ -254,6 +254,12 @@ STATIC void connection_ap_handshake_rewrite(entry_connection_t *conn, rewrite_result_t *out); STATIC int connection_ap_process_http_connect(entry_connection_t *conn); +struct half_edge_t; +STATIC void connection_half_edge_add(const edge_connection_t *conn, + origin_circuit_t *circ); +STATIC struct half_edge_t *connection_half_edge_find_stream_id( + const smartlist_t *half_conns, + streamid_t stream_id); #endif /* defined(CONNECTION_EDGE_PRIVATE) */ #endif /* !defined(TOR_CONNECTION_EDGE_H) */ diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c index 2d020ec472..e2d666d52e 100644 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@ -5,6 +5,9 @@ #define RELAY_PRIVATE #define CIRCUITLIST_PRIVATE +#define CONNECTION_EDGE_PRIVATE +#define CONNECTION_PRIVATE + #include "core/or/or.h" #include "core/mainloop/main.h" #include "app/config/config.h" @@ -25,6 +28,9 @@ #include "core/or/socks_request_st.h" #include "core/or/half_edge_st.h" +#include "feature/client/circpathbias.h" +#include "core/or/connection_edge.h" + static int srm_ncalls; static entry_connection_t *srm_conn; static int srm_atype; @@ -34,23 +40,6 @@ static uint8_t srm_answer[512]; static int srm_ttl; static time_t srm_expires; -void connection_free_minimal(connection_t*); -int connected_cell_format_payload(uint8_t *payload_out, - const tor_addr_t *addr, - uint32_t ttl); -void pathbias_count_valid_cells(origin_circuit_t *circ, - cell_t *cell); -half_edge_t *connection_half_edge_find_stream_id( - const smartlist_t *half_conns, - streamid_t stream_id); -void connection_half_edge_add(const edge_connection_t *conn, - origin_circuit_t *circ); - -int mock_send_command(streamid_t stream_id, circuit_t *circ, - uint8_t relay_command, const char *payload, - size_t payload_len, crypt_path_t *cpath_layer, - const char *filename, int lineno); - /* Mock replacement for connection_ap_hannshake_socks_resolved() */ static void socks_resolved_mock(entry_connection_t *conn, @@ -150,7 +139,7 @@ mock_start_reading(connection_t *conn) return; } -int +static int mock_send_command(streamid_t stream_id, circuit_t *circ, uint8_t relay_command, const char *payload, size_t payload_len, crypt_path_t *cpath_layer, @@ -237,7 +226,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) /* Data cell not in the half-opened list */ PACK_CELL(4000, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -246,7 +235,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) /* Sendme cell not in the half-opened list */ PACK_CELL(4000, RELAY_COMMAND_SENDME, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -255,7 +244,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) /* Connected cell not in the half-opened list */ PACK_CELL(4000, RELAY_COMMAND_CONNECTED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -264,7 +253,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) /* Resolved cell not in the half-opened list */ PACK_CELL(4000, RELAY_COMMAND_RESOLVED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -274,7 +263,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) edgeconn = ENTRY_TO_EDGE_CONN(entryconn2); PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_CONNECTED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -286,7 +275,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -297,7 +286,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -309,7 +298,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_SENDME, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -320,7 +309,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_SENDME, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -331,7 +320,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_END, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -341,7 +330,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_END, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -366,7 +355,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) connection_edge_reached_eof(edgeconn); PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_CONNECTED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -376,7 +365,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn3)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_CONNECTED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -387,7 +376,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn3)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_END, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -406,7 +395,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn3)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -426,7 +415,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_RESOLVED, "\x04\x04\x12\x00\x00\x01\x00\x00\x02\x00"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -445,7 +434,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn4)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -456,7 +445,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn4)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_END, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -877,7 +866,7 @@ test_circbw_relay(void *arg) /* Path bias: truncated */ tt_int_op(circ->base_.marked_for_close, OP_EQ, 0); PACK_CELL(0, RELAY_COMMAND_TRUNCATED, "Data1234"); - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); tt_int_op(circ->base_.marked_for_close, OP_EQ, 1); done: From dddecee291cadf391d93b569023f1f1e008880e8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Sep 2018 15:13:10 -0400 Subject: [PATCH 5/7] Initialize some locals in socks5 parsing code. These confused GCC LTO, which thought they might be used uninitialized. I'm pretty sure that as long as 'res' indicates success, they will always be set to something, but let's unconfuse the compiler in any case. --- src/core/proto/proto_socks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/proto/proto_socks.c b/src/core/proto/proto_socks.c index e2f233ad05..e23da7730b 100644 --- a/src/core/proto/proto_socks.c +++ b/src/core/proto/proto_socks.c @@ -744,7 +744,7 @@ handle_socks_message(const uint8_t *raw_data, size_t datalen, res = SOCKS_RESULT_MORE_EXPECTED; goto end; } else if (req->socks_version != SOCKS_VER_5) { - int have_user_pass, have_no_auth; + int have_user_pass=0, have_no_auth=0; res = parse_socks5_methods_request(raw_data, req, datalen, &have_user_pass, &have_no_auth, From 7c8f20ba44a831ed9f714453fa776762d1c872c5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Sep 2018 15:18:52 -0400 Subject: [PATCH 6/7] In tor_tls_get_my_certs(), set cert ptrs even on failure Nothing should ever look at them on failure, but in some cases, the unit tests don't check for failure, and then GCC-LTO freaks out. Fixes part of 27772. --- src/lib/tls/tortls.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/lib/tls/tortls.c b/src/lib/tls/tortls.c index 3ae3a1a096..56f70bc371 100644 --- a/src/lib/tls/tortls.c +++ b/src/lib/tls/tortls.c @@ -71,13 +71,19 @@ tor_tls_get_my_certs(int server, const tor_x509_cert_t **id_cert_out) { tor_tls_context_t *ctx = tor_tls_context_get(server); - if (! ctx) - return -1; + int rv = -1; + const tor_x509_cert_t *link_cert = NULL; + const tor_x509_cert_t *id_cert = NULL; + if (ctx) { + rv = 0; + link_cert = server ? ctx->my_link_cert : ctx->my_auth_cert; + id_cert = ctx->my_id_cert; + } if (link_cert_out) - *link_cert_out = server ? ctx->my_link_cert : ctx->my_auth_cert; + *link_cert_out = link_cert; if (id_cert_out) - *id_cert_out = ctx->my_id_cert; - return 0; + *id_cert_out = id_cert; + return rv; } /** From 6925b61cfdf50a5686de02645b04b269c031f05d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 18 Sep 2018 15:32:25 -0400 Subject: [PATCH 7/7] Fix various GCC LTO warnings in the unit tests. --- src/test/test_hs_service.c | 1 + src/test/test_routerset.c | 3 +++ src/test/test_storagedir.c | 2 +- src/test/test_tortls.c | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index bceeafd149..d6404bd715 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -534,6 +534,7 @@ test_load_keys_with_client_auth(void *arg) tt_int_op(get_hs_service_map_size(), OP_EQ, 1); service = get_first_service(); + tt_assert(service); tt_assert(service->config.clients); tt_int_op(smartlist_len(service->config.clients), OP_EQ, smartlist_len(pubkey_b32_list)); diff --git a/src/test/test_routerset.c b/src/test/test_routerset.c index 2017ef0050..db7a6a54ea 100644 --- a/src/test/test_routerset.c +++ b/src/test/test_routerset.c @@ -1496,6 +1496,7 @@ NS(test_main)(void *arg) int r; (void)arg; + memset(&NS(mock_node), 0, sizeof(NS(mock_node))); NS(mock_node).ri = NULL; NS(mock_node).rs = NULL; @@ -1529,6 +1530,7 @@ NS(test_main)(void *arg) strncpy(rs.nickname, nickname, sizeof(rs.nickname) - 1); rs.nickname[sizeof(rs.nickname) - 1] = '\0'; + memset(&NS(mock_node), 0, sizeof(NS(mock_node))); NS(mock_node).ri = NULL; NS(mock_node).rs = &rs; @@ -1560,6 +1562,7 @@ NS(test_main)(void *arg) strmap_set_lc(set->names, nickname, (void *)1); ri.nickname = (char *)nickname; + memset(&mock_node, 0, sizeof(mock_node)); mock_node.ri = &ri; mock_node.rs = NULL; diff --git a/src/test/test_storagedir.c b/src/test/test_storagedir.c index 68cee418ad..76aae7e033 100644 --- a/src/test/test_storagedir.c +++ b/src/test/test_storagedir.c @@ -283,7 +283,7 @@ test_storagedir_save_labeled(void *arg) int r = storage_dir_save_labeled_to_file(d, labels, inp, 8192, &fname); tt_int_op(r, OP_EQ, 0); - size_t n; + size_t n = 0; saved = storage_dir_read(d, fname, 1, &n); tt_assert(memchr(saved, '\0', n)); tt_str_op((char*)saved, OP_EQ, expected); /* NUL guarantees strcmp works */ diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c index f4315364a2..79b52437f8 100644 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@ -238,6 +238,7 @@ test_tortls_x509_cert_get_id_digests(void *ignored) cert->pkey_digests_set = 1; cert->pkey_digests = *d; res = tor_x509_cert_get_id_digests(cert); + tt_assert(res); tt_int_op(res->d[0][0], OP_EQ, 42); done: