Commit 707c1e2e authored by Andrea Shepard's avatar Andrea Shepard
Browse files

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

parent b4e8d8dc

changes/bug9602

0 → 100644
+4 −0
Original line number Diff line number Diff line 
 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.

src/or/channeltls.c

+124 −69
Original line number Diff line number Diff line
@@ -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);



  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,10 +429,9 @@ channel_tls_get_remote_descr_method(channel_t *chan, int flags) 
  char *addr_str;



  tor_assert(tlschan);

  tor_assert(tlschan->conn);



  if (tlschan->conn) {

    conn = TO_CONN(tlschan->conn);



    switch (flags) {

      case 0:

        /* Canonical address with port*/
@@ -457,11 +459,14 @@ channel_tls_get_remote_descr_method(channel_t *chan, int flags) 
        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,8 +514,8 @@ 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);



  if (tlschan->conn) {

    switch (req) {

      case 0:

        answer = tlschan->conn->is_canonical;
@@ -520,6 +532,8 @@ channel_tls_is_canonical_method(channel_t *chan, int req) 
        /* 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);



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



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

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



  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;

}



/*************************************************

src/or/connection.c

+16 −0
Original line number Diff line number Diff line
@@ -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);

src/or/connection_or.c

+5 −0
Original line number Diff line number Diff line
@@ -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;

  }