tor-commits April 2016

tor-commits@lists.torproject.org

20 participants
1645 discussions

[tor/release-0.2.7] changes file for bug18570
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit 72ebf4160412f64fb6ae0cd97dd89d01d89c075a Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 21 10:19:07 2016 -0400 changes file for bug18570 --- changes/bug18570 | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/changes/bug18570 b/changes/bug18570 new file mode 100644 index 0000000..04f72f4 --- /dev/null +++ b/changes/bug18570 @@ -0,0 +1,7 @@ + o Minor bugfixes (correctness): + - Fix a bad memory handling bug that would occur if we had queued + a cell on a channel's incoming queue. Fortunately, we can't actually + queue a cell like that as our code is constructed today, but it's best + to avoid this kind of error, even if there isn't any code that triggers + it today. Fixes bug 18570; bugfix on 0.2.4.4-alpha. +

1 0

[tor/release-0.2.7] Make sure channel_t queues its own copy of incoming cells
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit bd87d37a861c541afbeb660b4d8dd62df14d5b45 Author: Andrea Shepard <andrea(a)torproject.org> Date: Tue Mar 15 07:40:19 2016 +0000 Make sure channel_t queues its own copy of incoming cells --- src/or/channel.c | 38 ++++++++++++++++++++++++++++++++++---- src/or/channeltls.c | 11 +++++++++++ src/or/connection_or.c | 35 +++++++++++++++++++++++++++++++++++ src/or/connection_or.h | 1 + 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/or/channel.c b/src/or/channel.c index 21522a5..62a21be 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -2652,6 +2652,11 @@ channel_process_cells(channel_t *chan) /* * Process cells until we're done or find one we have no current handler * for. + * + * We must free the cells here after calling the handler, since custody + * of the buffer was given to the channel layer when they were queued; + * see comments on memory management in channel_queue_cell() and in + * channel_queue_var_cell() below. */ while (NULL != (q = TOR_SIMPLEQ_FIRST(&chan->incoming_queue))) { tor_assert(q); @@ -2669,6 +2674,7 @@ channel_process_cells(channel_t *chan) q->u.fixed.cell, chan, U64_PRINTF_ARG(chan->global_identifier)); chan->cell_handler(chan, q->u.fixed.cell); + tor_free(q->u.fixed.cell); tor_free(q); } else if (q->type == CELL_QUEUE_VAR && chan->var_cell_handler) { @@ -2681,6 +2687,7 @@ channel_process_cells(channel_t *chan) q->u.var.var_cell, chan, U64_PRINTF_ARG(chan->global_identifier)); chan->var_cell_handler(chan, q->u.var.var_cell); + tor_free(q->u.var.var_cell); tor_free(q); } else { /* Can't handle this one */ @@ -2701,6 +2708,7 @@ channel_queue_cell(channel_t *chan, cell_t *cell) { int need_to_queue = 0; cell_queue_entry_t *q; + cell_t *cell_copy = NULL; tor_assert(chan); tor_assert(cell); @@ -2728,8 +2736,19 @@ channel_queue_cell(channel_t *chan, cell_t *cell) U64_PRINTF_ARG(chan->global_identifier)); chan->cell_handler(chan, cell); } else { - /* Otherwise queue it and then process the queue if possible. */ - q = cell_queue_entry_new_fixed(cell); + /* + * Otherwise queue it and then process the queue if possible. + * + * We queue a copy, not the original pointer - it might have been on the + * stack in connection_or_process_cells_from_inbuf() (or another caller + * if we ever have a subclass other than channel_tls_t), or be freed + * there after we return. This is the uncommon case; the non-copying + * fast path occurs in the if (!need_to_queue) case above when the + * upper layer has installed cell handlers. + */ + cell_copy = tor_malloc_zero(sizeof(cell_t)); + memcpy(cell_copy, cell, sizeof(cell_t)); + q = cell_queue_entry_new_fixed(cell_copy); log_debug(LD_CHANNEL, "Queueing incoming cell_t %p for channel %p " "(global ID " U64_FORMAT ")", @@ -2755,6 +2774,7 @@ channel_queue_var_cell(channel_t *chan, var_cell_t *var_cell) { int need_to_queue = 0; cell_queue_entry_t *q; + var_cell_t *cell_copy = NULL; tor_assert(chan); tor_assert(var_cell); @@ -2783,8 +2803,18 @@ channel_queue_var_cell(channel_t *chan, var_cell_t *var_cell) U64_PRINTF_ARG(chan->global_identifier)); chan->var_cell_handler(chan, var_cell); } else { - /* Otherwise queue it and then process the queue if possible. */ - q = cell_queue_entry_new_var(var_cell); + /* + * Otherwise queue it and then process the queue if possible. + * + * We queue a copy, not the original pointer - it might have been on the + * stack in connection_or_process_cells_from_inbuf() (or another caller + * if we ever have a subclass other than channel_tls_t), or be freed + * there after we return. This is the uncommon case; the non-copying + * fast path occurs in the if (!need_to_queue) case above when the + * upper layer has installed cell handlers. + */ + cell_copy = var_cell_copy(var_cell); + q = cell_queue_entry_new_var(cell_copy); log_debug(LD_CHANNEL, "Queueing incoming var_cell_t %p for channel %p " "(global ID " U64_FORMAT ")", diff --git a/src/or/channeltls.c b/src/or/channeltls.c index c90f569..2a84514 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1009,6 +1009,11 @@ channel_tls_time_process_cell(cell_t *cell, channel_tls_t *chan, int *time, * for cell types specific to the handshake for this transport protocol and * handles them, and queues all other cells to the channel_t layer, which * eventually will hand them off to command.c. + * + * The channel layer itself decides whether the cell should be queued or + * can be handed off immediately to the upper-layer code. It is responsible + * for copying in the case that it queues; we merely pass pointers through + * which we get from connection_or_process_cells_from_inbuf(). */ void @@ -1106,6 +1111,12 @@ channel_tls_handle_cell(cell_t *cell, or_connection_t *conn) * related and live below the channel_t layer, so no variable-length * cells ever get delivered in the current implementation, but I've left * the mechanism in place for future use. + * + * If we were handing them off to the upper layer, the channel_t queueing + * code would be responsible for memory management, and we'd just be passing + * pointers through from connection_or_process_cells_from_inbuf(). That + * caller always frees them after this function returns, so this function + * should never free var_cell. */ void diff --git a/src/or/connection_or.c b/src/or/connection_or.c index a967c93..9944494 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -488,6 +488,28 @@ var_cell_new(uint16_t payload_len) return cell; } +/** + * Copy a var_cell_t + */ + +var_cell_t * +var_cell_copy(const var_cell_t *src) +{ + var_cell_t *copy = NULL; + size_t size = 0; + + if (src != NULL) { + size = STRUCT_OFFSET(var_cell_t, payload) + src->payload_len; + copy = tor_malloc_zero(size); + copy->payload_len = src->payload_len; + copy->command = src->command; + copy->circ_id = src->circ_id; + memcpy(copy->payload, src->payload, copy->payload_len); + } + + return copy; +} + /** Release all space held by cell. */ void var_cell_free(var_cell_t *cell) @@ -2060,6 +2082,19 @@ connection_or_process_cells_from_inbuf(or_connection_t *conn) { var_cell_t *var_cell; + /* + * Note on memory management for incoming cells: below the channel layer, + * we shouldn't need to consider its internal queueing/copying logic. It + * is safe to pass cells to it on the stack or on the heap, but in the + * latter case we must be sure we free them later. + * + * The incoming cell queue code in channel.c will (in the common case) + * decide it can pass them to the upper layer immediately, in which case + * those functions may run directly on the cell pointers we pass here, or + * it may decide to queue them, in which case it will allocate its own + * buffer and copy the cell. + */ + while (1) { log_debug(LD_OR, TOR_SOCKET_T_FORMAT": starting, inbuf_datalen %d " diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 3877fd5..0bd8567 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -97,6 +97,7 @@ void cell_pack(packed_cell_t *dest, const cell_t *src, int wide_circ_ids); int var_cell_pack_header(const var_cell_t *cell, char *hdr_out, int wide_circ_ids); var_cell_t *var_cell_new(uint16_t payload_len); +var_cell_t *var_cell_copy(const var_cell_t *src); void var_cell_free(var_cell_t *cell); /** DOCDOC */

1 0

[tor/release-0.2.7] Merge branch 'bug16248_027' into maint-0.2.7
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit 1218d731d1f3c1944a22bba45ef2f6a0853eba5c Merge: e1e62f9 307b863 Author: Andrea Shepard <andrea(a)torproject.org> Date: Tue Mar 29 14:33:45 2016 +0000 Merge branch 'bug16248_027' into maint-0.2.7 changes/bug16248 | 8 ++++++++ src/or/main.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 4 deletions(-)

1 0

[tor/release-0.2.7] Merge branch 'bug15221_027' into maint-0.2.7
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit 183d465f0e497cbc5f49c08119dfdf00eb7f8cdd Merge: 0b45cab 725e0c7 Author: Andrea Shepard <andrea(a)torproject.org> Date: Wed Mar 30 12:23:42 2016 +0000 Merge branch 'bug15221_027' into maint-0.2.7 changes/bug15221 | 4 ++++ src/common/sandbox.c | 9 +++++++++ 2 files changed, 13 insertions(+)

1 0

[tor/release-0.2.7] Merge branch 'bug18570_027' into maint-0.2.7
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit 0b45cab1477d93e3065ca2d35e87483688f16bf6 Merge: 1218d73 72ebf41 Author: Andrea Shepard <andrea(a)torproject.org> Date: Tue Mar 29 15:01:36 2016 +0000 Merge branch 'bug18570_027' into maint-0.2.7 changes/bug18570 | 7 +++ src/or/channel.c | 38 +++++++++++++-- src/or/channeltls.c | 11 +++++ src/or/connection_or.c | 35 ++++++++++++++ src/or/connection_or.h | 1 + src/test/test_channel.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 204 insertions(+), 9 deletions(-)

1 0

[tor/release-0.2.6] Merge branch 'maint-0.2.4' into maint-0.2.5
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit f06d9a9cef036fc4da995f3ffda8e7a7770d0b74 Merge: 9ca3295 d5ac79e Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Feb 5 08:13:13 2016 -0500 Merge branch 'maint-0.2.4' into maint-0.2.5 changes/geoip-february2016 | 4 + src/config/geoip | 2144 +++++++++++++++++++++++++++++++++----------- src/config/geoip6 | 864 ++++++++++++++---- 3 files changed, 2328 insertions(+), 684 deletions(-)

1 0

[tor/release-0.2.6] Merge branch 'bug18162_024' into maint-0.2.4
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit ad95d64fece2c6d2eddffc8fa5178c3ffccc0cd7 Merge: d5ac79e c2fd648 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 11 12:55:25 2016 -0500 Merge branch 'bug18162_024' into maint-0.2.4 changes/bug18162 | 7 +++++++ src/common/container.c | 29 +++++++++++++++++++---------- 2 files changed, 26 insertions(+), 10 deletions(-)

1 0

[tor/release-0.2.6] Merge branch 'maint-0.2.5' into maint-0.2.6
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit 44ad3be221fb8e5e648c6d56054fb1d8596a645b Merge: e2efa9e f06d9a9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Feb 5 08:13:24 2016 -0500 Merge branch 'maint-0.2.5' into maint-0.2.6 changes/geoip-february2016 | 4 + src/config/geoip | 2144 +++++++++++++++++++++++++++++++++----------- src/config/geoip6 | 864 ++++++++++++++---- 3 files changed, 2328 insertions(+), 684 deletions(-)

1 0

[tor/release-0.2.6] Make ensure_capacity a bit more pedantically correct
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit c2fd64846978290b0e7c7165d7658a5e704eee8f Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 11 12:54:52 2016 -0500 Make ensure_capacity a bit more pedantically correct Issues noted by cypherpunks on #18162 --- src/common/container.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/common/container.c b/src/common/container.c index 46d9c2e..b1431df 100644 --- a/src/common/container.c +++ b/src/common/container.c @@ -58,11 +58,16 @@ smartlist_clear(smartlist_t *sl) sl->num_used = 0; } +#if SIZE_MAX < INT_MAX +#error "We don't support systems where size_t is smaller than int." +#endif + /** Make sure that sl can hold at least size entries. */ static INLINE void smartlist_ensure_capacity(smartlist_t *sl, size_t size) { -#if SIZEOF_SIZE_T > SIZEOF_INT + /* Set MAX_CAPACITY to MIN(INT_MAX, SIZE_MAX / sizeof(void*)) */ +#if (SIZE_MAX/SIZEOF_VOID_P) > INT_MAX #define MAX_CAPACITY (INT_MAX) #else #define MAX_CAPACITY (int)((SIZE_MAX / (sizeof(void*))))

1 0

[tor/release-0.2.6] Merge branch 'maint-0.2.4' into maint-0.2.5
by nickm＠torproject.org 07 Apr '16

07 Apr '16

commit ce289e2cb5099a4abe4468049dc0d262b65bfa17 Merge: f06d9a9 ad95d64 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 11 12:55:40 2016 -0500 Merge branch 'maint-0.2.4' into maint-0.2.5 changes/bug18162 | 7 +++++++ src/common/container.c | 29 +++++++++++++++++++---------- 2 files changed, 26 insertions(+), 10 deletions(-)

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits April 2016