commit 0748c06f7c2a44fc1cc378d3afadeebd894c493c Author: Nick Mathewson nickm@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 */