[tor-bugs] #6480 [Tor Relay]: double connection_free() in dns_resolve()

[Tor Bug Tracker & Wiki]

#6480: double connection_free() in dns_resolve()
-----------------------+----------------------------------------------------
 Reporter:  arma       |          Owner:                    
     Type:  defect     |         Status:  needs_review      
 Priority:  normal     |      Milestone:  Tor: 0.2.4.x-final
Component:  Tor Relay  |        Version:                    
 Keywords:             |         Parent:                    
   Points:             |   Actualpoints:                    
-----------------------+----------------------------------------------------
Changes (by arma):

  * status:  new => needs_review


Comment:

 Our friendly irc person suggests this patch:
 {{{
 --- src/or/dns.c
 +++ src/or/dns.mod.c
 @@ -168,7 +168,8 @@
  static int configure_nameservers(int force);
  static int answer_is_wildcarded(const char *ip);
  static int dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
 -                            or_circuit_t *oncirc, char
 **resolved_to_hostname);
 +                            or_circuit_t *oncirc, char
 **resolved_to_hostname,
 +                            int *pended_connection);
  #ifdef DEBUG_DNS_CACHE
  static void _assert_cache_ok(void);
  #define assert_cache_ok() _assert_cache_ok()
 @@ -597,9 +598,11 @@
    or_circuit_t *oncirc = TO_OR_CIRCUIT(exitconn->on_circuit);
    int is_resolve, r;
    char *hostname = NULL;
 +  int pended_connection = 0;
    is_resolve = exitconn->_base.purpose == EXIT_PURPOSE_RESOLVE;

 -  r = dns_resolve_impl(exitconn, is_resolve, oncirc, &hostname);
 +  r = dns_resolve_impl(exitconn, is_resolve, oncirc, &hostname,
 +                       &pended_connection);

    switch (r) {
      case 1:
 @@ -639,7 +642,7 @@

        dns_cancel_pending_resolve(exitconn->_base.address);

 -      if (!exitconn->_base.marked_for_close) {
 +      if (!pended_connection && !exitconn->_base.marked_for_close) {
          connection_free(TO_CONN(exitconn));
          // XXX ... and we just leak exitconn otherwise? -RD
          // If it's marked for close, it's on closeable_connection_lst in
 @@ -670,7 +673,8 @@
   */
  static int
  dns_resolve_impl(edge_connection_t *exitconn, int is_resolve,
 -                 or_circuit_t *oncirc, char **hostname_out)
 +                 or_circuit_t *oncirc, char **hostname_out,
 +                 int *pended_connection)
  {
    cached_resolve_t *resolve;
    cached_resolve_t search;
 @@ -797,6 +801,7 @@
    pending_connection = tor_malloc_zero(sizeof(pending_connection_t));
    pending_connection->conn = exitconn;
    resolve->pending_connections = pending_connection;
 +  *pended_connection = 1;

    /* Add this resolve to the cache and priority queue. */
    HT_INSERT(cache_map, &cache_root, resolve);
 }}}

 which looks pretty straightforward.

 I assigned the ticket to 0.2.4 originally since it isn't occurring in
 practice. We might want to move that to 0.2.3 if we become confident of
 the diagnosis and fix.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/6480#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online