[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/release-0.2.4] NULL out conns on tlschans when freeing in case channel_run_cleanup() is late; fixes bug 9602
commit 707c1e2e263fd34f70a5f780e77820d667ba2931
Author: Andrea Shepard <andrea@xxxxxxxxxxxxxx>
Date: Thu Feb 6 14:47:34 2014 -0800
NULL out conns on tlschans when freeing in case channel_run_cleanup() is late; fixes bug 9602
---
changes/bug9602 | 4 +
src/or/channeltls.c | 193 +++++++++++++++++++++++++++++++-----------------
src/or/connection.c | 16 ++++
src/or/connection_or.c | 5 ++
4 files changed, 149 insertions(+), 69 deletions(-)
diff --git a/changes/bug9602 b/changes/bug9602
new file mode 100644
index 0000000..9e9a3ca
--- /dev/null
+++ b/changes/bug9602
@@ -0,0 +1,4 @@
+ o Bugfixes
+ - Null out orconn->chan->conn when closing orconn in case orconn is freed
+ before channel_run_cleanup() gets to orconn->chan, and handle the null
+ conn edge case correctly in channel_tls_t methods. Fixes bug #9602.
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index f751c0d..495f856 100644
--- a/src/or/channeltls.c
+++ b/src/or/channeltls.c
@@ -365,7 +365,7 @@ channel_tls_describe_transport_method(channel_t *chan)
tor_assert(chan);
- tlschan = BASE_CHAN_TO_TLS(chan);
+ tlschan = BASE_CHAN_TO_TLS(chan);
if (tlschan->conn) {
id = TO_CONN(tlschan->conn)->global_identifier;
@@ -394,15 +394,18 @@ channel_tls_describe_transport_method(channel_t *chan)
static int
channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out)
{
+ int rv = 0;
channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
tor_assert(tlschan);
tor_assert(addr_out);
- tor_assert(tlschan->conn);
- tor_addr_copy(addr_out, &(TO_CONN(tlschan->conn)->addr));
+ if (tlschan->conn) {
+ tor_addr_copy(addr_out, &(TO_CONN(tlschan->conn)->addr));
+ rv = 1;
+ } else tor_addr_make_unspec(addr_out);
- return 1;
+ return rv;
}
/**
@@ -426,41 +429,43 @@ channel_tls_get_remote_descr_method(channel_t *chan, int flags)
char *addr_str;
tor_assert(tlschan);
- tor_assert(tlschan->conn);
-
- conn = TO_CONN(tlschan->conn);
- switch (flags) {
- case 0:
- /* Canonical address with port*/
- tor_snprintf(buf, MAX_DESCR_LEN + 1,
- "%s:%u", conn->address, conn->port);
- answer = buf;
- break;
- case GRD_FLAG_ORIGINAL:
- /* Actual address with port */
- addr_str = tor_dup_addr(&(tlschan->conn->real_addr));
- tor_snprintf(buf, MAX_DESCR_LEN + 1,
- "%s:%u", addr_str, conn->port);
- tor_free(addr_str);
- answer = buf;
- break;
- case GRD_FLAG_ADDR_ONLY:
- /* Canonical address, no port */
- strlcpy(buf, conn->address, sizeof(buf));
- answer = buf;
- break;
- case GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY:
- /* Actual address, no port */
- addr_str = tor_dup_addr(&(tlschan->conn->real_addr));
- strlcpy(buf, addr_str, sizeof(buf));
- tor_free(addr_str);
- answer = buf;
- break;
-
- default:
- /* Something's broken in channel.c */
- tor_assert(1);
+ if (tlschan->conn) {
+ conn = TO_CONN(tlschan->conn);
+ switch (flags) {
+ case 0:
+ /* Canonical address with port*/
+ tor_snprintf(buf, MAX_DESCR_LEN + 1,
+ "%s:%u", conn->address, conn->port);
+ answer = buf;
+ break;
+ case GRD_FLAG_ORIGINAL:
+ /* Actual address with port */
+ addr_str = tor_dup_addr(&(tlschan->conn->real_addr));
+ tor_snprintf(buf, MAX_DESCR_LEN + 1,
+ "%s:%u", addr_str, conn->port);
+ tor_free(addr_str);
+ answer = buf;
+ break;
+ case GRD_FLAG_ADDR_ONLY:
+ /* Canonical address, no port */
+ strlcpy(buf, conn->address, sizeof(buf));
+ answer = buf;
+ break;
+ case GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY:
+ /* Actual address, no port */
+ addr_str = tor_dup_addr(&(tlschan->conn->real_addr));
+ strlcpy(buf, addr_str, sizeof(buf));
+ tor_free(addr_str);
+ answer = buf;
+ break;
+ default:
+ /* Something's broken in channel.c */
+ tor_assert(1);
+ }
+ } else {
+ strlcpy(buf, "(No connection)", sizeof(buf));
+ answer = buf;
}
return answer;
@@ -480,9 +485,16 @@ channel_tls_has_queued_writes_method(channel_t *chan)
channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
tor_assert(tlschan);
- tor_assert(tlschan->conn);
+ if (!(tlschan->conn)) {
+ log_info(LD_CHANNEL,
+ "something called has_queued_writes on a tlschan "
+ "(%p with ID " U64_FORMAT " but no conn",
+ chan, U64_PRINTF_ARG(chan->global_identifier));
+ }
- outbuf_len = connection_get_outbuf_len(TO_CONN(tlschan->conn));
+ outbuf_len = (tlschan->conn != NULL) ?
+ connection_get_outbuf_len(TO_CONN(tlschan->conn)) :
+ 0;
return (outbuf_len > 0);
}
@@ -502,24 +514,26 @@ channel_tls_is_canonical_method(channel_t *chan, int req)
channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
tor_assert(tlschan);
- tor_assert(tlschan->conn);
- switch (req) {
- case 0:
- answer = tlschan->conn->is_canonical;
- break;
- case 1:
- /*
- * Is the is_canonical bit reliable? In protocols version 2 and up
- * we get the canonical address from a NETINFO cell, but in older
- * versions it might be based on an obsolete descriptor.
- */
- answer = (tlschan->conn->link_proto >= 2);
- break;
- default:
- /* This shouldn't happen; channel.c is broken if it does */
- tor_assert(1);
+ if (tlschan->conn) {
+ switch (req) {
+ case 0:
+ answer = tlschan->conn->is_canonical;
+ break;
+ case 1:
+ /*
+ * Is the is_canonical bit reliable? In protocols version 2 and up
+ * we get the canonical address from a NETINFO cell, but in older
+ * versions it might be based on an obsolete descriptor.
+ */
+ answer = (tlschan->conn->link_proto >= 2);
+ break;
+ default:
+ /* This shouldn't happen; channel.c is broken if it does */
+ tor_assert(1);
+ }
}
+ /* else return 0 for tlschan->conn == NULL */
return answer;
}
@@ -540,6 +554,15 @@ channel_tls_matches_extend_info_method(channel_t *chan,
tor_assert(tlschan);
tor_assert(extend_info);
+ /* Never match if we have no conn */
+ if (!(tlschan->conn)) {
+ log_info(LD_CHANNEL,
+ "something called matches_extend_info on a tlschan "
+ "(%p with ID " U64_FORMAT " but no conn",
+ chan, U64_PRINTF_ARG(chan->global_identifier));
+ return 0;
+ }
+
return (tor_addr_eq(&(extend_info->addr),
&(TO_CONN(tlschan->conn)->addr)) &&
(extend_info->port == TO_CONN(tlschan->conn)->port));
@@ -561,7 +584,15 @@ channel_tls_matches_target_method(channel_t *chan,
tor_assert(tlschan);
tor_assert(target);
- tor_assert(tlschan->conn);
+
+ /* Never match if we have no conn */
+ if (!(tlschan->conn)) {
+ log_info(LD_CHANNEL,
+ "something called matches_target on a tlschan "
+ "(%p with ID " U64_FORMAT " but no conn",
+ chan, U64_PRINTF_ARG(chan->global_identifier));
+ return 0;
+ }
return tor_addr_eq(&(tlschan->conn->real_addr), target);
}
@@ -577,14 +608,22 @@ static int
channel_tls_write_cell_method(channel_t *chan, cell_t *cell)
{
channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
+ int written = 0;
tor_assert(tlschan);
tor_assert(cell);
- tor_assert(tlschan->conn);
- connection_or_write_cell_to_buf(cell, tlschan->conn);
+ if (tlschan->conn) {
+ connection_or_write_cell_to_buf(cell, tlschan->conn);
+ ++written;
+ } else {
+ log_info(LD_CHANNEL,
+ "something called write_cell on a tlschan "
+ "(%p with ID " U64_FORMAT " but no conn",
+ chan, U64_PRINTF_ARG(chan->global_identifier));
+ }
- return 1;
+ return written;
}
/**
@@ -600,18 +639,26 @@ channel_tls_write_packed_cell_method(channel_t *chan,
{
channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
size_t cell_network_size = get_cell_network_size(chan->wide_circ_ids);
+ int written = 0;
tor_assert(tlschan);
tor_assert(packed_cell);
- tor_assert(tlschan->conn);
- connection_write_to_buf(packed_cell->body, cell_network_size,
- TO_CONN(tlschan->conn));
+ if (tlschan->conn) {
+ connection_write_to_buf(packed_cell->body, cell_network_size,
+ TO_CONN(tlschan->conn));
- /* This is where the cell is finished; used to be done from relay.c */
- packed_cell_free(packed_cell);
+ /* This is where the cell is finished; used to be done from relay.c */
+ packed_cell_free(packed_cell);
+ ++written;
+ } else {
+ log_info(LD_CHANNEL,
+ "something called write_packed_cell on a tlschan "
+ "(%p with ID " U64_FORMAT " but no conn",
+ chan, U64_PRINTF_ARG(chan->global_identifier));
+ }
- return 1;
+ return written;
}
/**
@@ -625,14 +672,22 @@ static int
channel_tls_write_var_cell_method(channel_t *chan, var_cell_t *var_cell)
{
channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
+ int written = 0;
tor_assert(tlschan);
tor_assert(var_cell);
- tor_assert(tlschan->conn);
- connection_or_write_var_cell_to_buf(var_cell, tlschan->conn);
+ if (tlschan->conn) {
+ connection_or_write_var_cell_to_buf(var_cell, tlschan->conn);
+ ++written;
+ } else {
+ log_info(LD_CHANNEL,
+ "something called write_var_cell on a tlschan "
+ "(%p with ID " U64_FORMAT " but no conn",
+ chan, U64_PRINTF_ARG(chan->global_identifier));
+ }
- return 1;
+ return written;
}
/*************************************************
diff --git a/src/or/connection.c b/src/or/connection.c
index 062c971..4f74a1d 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -514,6 +514,22 @@ connection_free_(connection_t *conn)
or_handshake_state_free(or_conn->handshake_state);
or_conn->handshake_state = NULL;
tor_free(or_conn->nickname);
+ if (or_conn->chan) {
+ /* Owww, this shouldn't happen, but... */
+ log_info(LD_CHANNEL,
+ "Freeing orconn at %p, saw channel %p with ID "
+ U64_FORMAT " left un-NULLed",
+ or_conn, TLS_CHAN_TO_BASE(or_conn->chan),
+ U64_PRINTF_ARG(
+ TLS_CHAN_TO_BASE(or_conn->chan)->global_identifier));
+ if (!(TLS_CHAN_TO_BASE(or_conn->chan)->state == CHANNEL_STATE_CLOSED ||
+ TLS_CHAN_TO_BASE(or_conn->chan)->state == CHANNEL_STATE_ERROR)) {
+ channel_close_for_error(TLS_CHAN_TO_BASE(or_conn->chan));
+ }
+
+ or_conn->chan->conn = NULL;
+ or_conn->chan = NULL;
+ }
}
if (conn->type == CONN_TYPE_AP) {
entry_connection_t *entry_conn = TO_ENTRY_CONN(conn);
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 3d16e14..8e7cd9e 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -622,6 +622,11 @@ connection_or_about_to_close(or_connection_t *or_conn)
/* Tell the controlling channel we're closed */
if (or_conn->chan) {
channel_closed(TLS_CHAN_TO_BASE(or_conn->chan));
+ /*
+ * NULL this out because the channel might hang around a little
+ * longer before channel_run_cleanup() gets it.
+ */
+ or_conn->chan->conn = NULL;
or_conn->chan = NULL;
}
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits