mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-28 06:13:31 +01:00
NULL out conns on tlschans when freeing in case channel_run_cleanup() is late; fixes bug 9602
This commit is contained in:
parent
b4e8d8dc0e
commit
707c1e2e26
4
changes/bug9602
Normal file
4
changes/bug9602
Normal 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.
|
@ -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;
|
||||
}
|
||||
|
||||
/*************************************************
|
||||
|
@ -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);
|
||||
|
@ -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;
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user