[tor-bugs] #18116 [Tor]: abort in circuitmux_detach_all_circuits on Tor Browser exit with stem scripts running

[Tor Bug Tracker & Wiki]

#18116: abort in circuitmux_detach_all_circuits on Tor Browser exit with stem
scripts running
-------------------------------------+------------------------------------
 Reporter:  teor                     |          Owner:  andrea
     Type:  defect                   |         Status:  assigned
 Priority:  Medium                   |      Milestone:  Tor: 0.2.8.x-final
Component:  Tor                      |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:  crash TorCoreTeam201602  |  Actual Points:
Parent ID:  #18117                   |         Points:
  Sponsor:                           |
-------------------------------------+------------------------------------

Comment (by andrea):

 The warning originates on circuitmux.c:488, BTW, also in
 circuitmux_detach_all_circuits():

 {{{
  391 /**
  392  * Detach all circuits from a circuitmux (use before
 circuitmux_free())
  393  *
  394  * If <b>detached_out</b> is non-NULL, add every detached circuit_t
 to
  395  * detached_out.
  396  */
  397
  398 void
  399 circuitmux_detach_all_circuits(circuitmux_t *cmux, smartlist_t
 *detached_out)
  400 {
  401   chanid_circid_muxinfo_t **i = NULL, *to_remove;
  402   channel_t *chan = NULL;
  403   circuit_t *circ = NULL;
  404
  405   tor_assert(cmux);
  406   /*
  407    * Don't circuitmux_assert_okay_paranoid() here; this gets called
 when
  408    * channels are being freed and have already been unregistered, so
  409    * the channel ID lookups it does will fail.
  410    */
  411
  412   i = HT_START(chanid_circid_muxinfo_map, cmux->chanid_circid_map);
  413   while (i) {
  414     to_remove = *i;
  415
  416     if (! to_remove) {
  417       log_warn(LD_BUG, "Somehow, an HT iterator gave us a NULL
 pointer.");
  418       break;
  419     } else {
  420       /* Find a channel and circuit */
  421       chan = channel_find_by_global_id(to_remove->chan_id);
  422       if (chan) {
  423         circ =
  424
 circuit_get_by_circid_channel_even_if_marked(to_remove->circ_id,
  425                                                        chan);
  426         if (circ) {
  427           /* Clear the circuit's mux for this direction */
  428           if (to_remove->muxinfo.direction == CELL_DIRECTION_OUT) {
  429             /*
  430              * Update active_circuits et al.; this does policy
 notifies, so
  431              * comes before freeing policy data
  432              */
  433
  434             if (to_remove->muxinfo.cell_count > 0) {
  435               circuitmux_make_circuit_inactive(cmux, circ,
 CELL_DIRECTION_OUT);
  436             }
  437
  438             /* Clear n_mux */
  439             circ->n_mux = NULL;
  440
  441             if (detached_out)
  442               smartlist_add(detached_out, circ);
  443           } else if (circ->magic == OR_CIRCUIT_MAGIC) {
  444             /*
  445              * Update active_circuits et al.; this does policy
 notifies, so
  446              * comes before freeing policy data
  447              */
  448
  449             if (to_remove->muxinfo.cell_count > 0) {
  450               circuitmux_make_circuit_inactive(cmux, circ,
 CELL_DIRECTION_IN);
  451             }
  452
  453             /*
  454              * It has a sensible p_chan and direction ==
 CELL_DIRECTION_IN,
  455              * so clear p_mux.
  456              */
  457             TO_OR_CIRCUIT(circ)->p_mux = NULL;
  458
  459             if (detached_out)
  460               smartlist_add(detached_out, circ);
  461           } else {
  462             /* Complain and move on */
  463             log_warn(LD_CIRC,
  464                      "Circuit %u/channel " U64_FORMAT " had direction
 == "
  465                      "CELL_DIRECTION_IN, but isn't an or_circuit_t",
  466                      (unsigned)to_remove->circ_id,
  467                      U64_PRINTF_ARG(to_remove->chan_id));
  468           }
  469
  470           /* Free policy-specific data if we have it */
  471           if (to_remove->muxinfo.policy_data) {
  472             /*
  473              * If we have policy data, assert that we have the means
 to
  474              * free it
  475              */
  476             tor_assert(cmux->policy);
  477             tor_assert(cmux->policy->free_circ_data);
  478             /* Call free_circ_data() */
  479             cmux->policy->free_circ_data(cmux,
  480                                          cmux->policy_data,
  481                                          circ,
  482
 to_remove->muxinfo.policy_data);
  483             to_remove->muxinfo.policy_data = NULL;
  484           }
  485         } else {
  486           /* Complain and move on */
  487           log_warn(LD_CIRC,
  488                    "Couldn't find circuit %u (for channel "
 U64_FORMAT ")",
  489                    (unsigned)to_remove->circ_id,
  490                    U64_PRINTF_ARG(to_remove->chan_id));
  491         }
  492       } else {
  493         /* Complain and move on */
  494         log_warn(LD_CIRC,
  495                  "Couldn't find channel " U64_FORMAT " (for circuit
 id %u)",
  496                  U64_PRINTF_ARG(to_remove->chan_id),
  497                  (unsigned)to_remove->circ_id);
  498       }
  499
  500       /* Assert that we don't have un-freed policy data for this
 circuit */
  501       tor_assert(to_remove->muxinfo.policy_data == NULL);
  502     }
  503
  504     i = HT_NEXT_RMV(chanid_circid_muxinfo_map,
 cmux->chanid_circid_map, i);
  505
  506     /* Free it */
  507     tor_free(to_remove);
  508   }
  509
  510   cmux->n_circuits = 0;
  511   cmux->n_active_circuits = 0;
  512   cmux->n_cells = 0;
  513 }
 }}}

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