[tor-commits] [tor/master] Fix bug 9082: avoid leak when freeing destroy cell queues

[nickm at torproject.org]

commit 0748c06f7c2a44fc1cc378d3afadeebd894c493c
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Jun 17 11:30:56 2013 -0400

    Fix bug 9082: avoid leak when freeing destroy cell queues
    
    In my #7912 fix, there wasn't any code to remove entries from the
    (channel, circuit ID)->circuit map corresponding to queued but un-sent
    DESTROYs.
    
    Spotted by skruffy. Fixes bug 9082; bug not in any released Tor.
---
 src/or/channel.c    |   37 +++++++++++++++++++++++++------------
 src/or/channel.h    |    4 ++++
 src/or/circuitmux.c |   25 +++++++++++++++++++++++++
 src/or/circuitmux.h |    2 ++
 4 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/src/or/channel.c b/src/or/channel.c
index 33a3210..98c23d9 100644
--- a/src/or/channel.c
+++ b/src/or/channel.c
@@ -803,6 +803,7 @@ channel_free(channel_t *chan)
   /* Get rid of cmux */
   if (chan->cmux) {
     circuitmux_detach_all_circuits(chan->cmux);
+    circuitmux_mark_destroyed_circids_usable(chan->cmux, chan);
     circuitmux_free(chan->cmux);
     chan->cmux = NULL;
   }
@@ -2616,6 +2617,29 @@ channel_queue_var_cell(channel_t *chan, var_cell_t *var_cell)
   }
 }
 
+/** If <b>packed_cell</b> on <b>chan</b> is a destroy cell, then set
+ * *<b>circid_out</b> to its circuit ID, and return true.  Otherwise, return
+ * false. */
+/* XXXX Move this function. */
+int
+packed_cell_is_destroy(channel_t *chan,
+                       const packed_cell_t *packed_cell,
+                       circid_t *circid_out)
+{
+  if (chan->wide_circ_ids) {
+    if (packed_cell->body[4] == CELL_DESTROY) {
+      *circid_out = ntohl(get_uint32(packed_cell->body));
+      return 1;
+    }
+  } else {
+    if (packed_cell->body[2] == CELL_DESTROY) {
+      *circid_out = ntohs(get_uint16(packed_cell->body));
+      return 1;
+    }
+  }
+  return 0;
+}
+
 /** DOCDOC */
 static int
 is_destroy_cell(channel_t *chan,
@@ -2636,18 +2660,7 @@ is_destroy_cell(channel_t *chan,
       }
       break;
     case CELL_QUEUE_PACKED:
-      if (chan->wide_circ_ids) {
-        if (q->u.packed.packed_cell->body[4] == CELL_DESTROY) {
-          *circid_out = ntohl(get_uint32(q->u.packed.packed_cell->body));
-          return 1;
-        }
-      } else {
-        if (q->u.packed.packed_cell->body[2] == CELL_DESTROY) {
-          *circid_out = ntohs(get_uint16(q->u.packed.packed_cell->body));
-          return 1;
-        }
-      }
-      break;
+      return packed_cell_is_destroy(chan, q->u.packed.packed_cell, circid_out);
   }
   return 0;
 }
diff --git a/src/or/channel.h b/src/or/channel.h
index 0933ec8..83d7e90 100644
--- a/src/or/channel.h
+++ b/src/or/channel.h
@@ -477,5 +477,9 @@ uint64_t channel_count_xmitted(channel_t *chan);
 
 uint64_t channel_listener_count_accepted(channel_listener_t *chan_l);
 
+int packed_cell_is_destroy(channel_t *chan,
+                           const packed_cell_t *packed_cell,
+                           circid_t *circid_out);
+
 #endif
 
diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c
index f579c3e..c84e0ce 100644
--- a/src/or/circuitmux.c
+++ b/src/or/circuitmux.c
@@ -498,6 +498,31 @@ circuitmux_detach_all_circuits(circuitmux_t *cmux)
   cmux->n_cells = 0;
 }
 
+/** Reclaim all circuit IDs currently marked as unusable on <b>chan</b> because
+ * of pending destroy cells in <b>cmux</b>.
+ *
+ * This function must be called AFTER circuits are unlinked from the (channel,
+ * circuid-id) map with circuit_unlink_all_from_channel(), but before calling
+ * circuitmux_free().
+ */
+void
+circuitmux_mark_destroyed_circids_usable(circuitmux_t *cmux, channel_t *chan)
+{
+  packed_cell_t *cell;
+  int n_bad = 0;
+  for (cell = cmux->destroy_cell_queue.head; cell; cell = cell->next) {
+    circid_t circid = 0;
+    if (packed_cell_is_destroy(chan, cell, &circid)) {
+      channel_mark_circid_usable(chan, circid);
+    } else {
+      ++n_bad;
+    }
+  }
+  if (n_bad)
+    log_warn(LD_BUG, "%d cell(s) on destroy queue did not look like a "
+             "DESTROY cell.", n_bad);
+}
+
 /**
  * Free a circuitmux_t; the circuits must be detached first with
  * circuitmux_detach_all_circuits().
diff --git a/src/or/circuitmux.h b/src/or/circuitmux.h
index 9ff29de..a6bc415 100644
--- a/src/or/circuitmux.h
+++ b/src/or/circuitmux.h
@@ -137,6 +137,8 @@ void circuitmux_set_num_cells(circuitmux_t *cmux, circuit_t *circ,
 void circuitmux_append_destroy_cell(channel_t *chan,
                                     circuitmux_t *cmux, circid_t circ_id,
                                     uint8_t reason);
+void circuitmux_mark_destroyed_circids_usable(circuitmux_t *cmux,
+                                              channel_t *chan);
 
 #endif /* TOR_CIRCUITMUX_H */