sarah
/
tor
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

NULL out conns on tlschans when freeing in case channel_run_cleanup() is late; fixes bug 9602

This commit is contained in:
Andrea Shepard 2014-02-06 14:47:34 -08:00
parent b4e8d8dc0e
commit 707c1e2e26
4 changed files with 149 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
changes/bug9602 Normal file
View File

@ -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.

193
src/or/channeltls.c
View File

@ -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;
}
/*************************************************

16
src/or/connection.c
View File

@ -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);

5
src/or/connection_or.c
View File

@ -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;
}