commit aba25a6939a5907d40dbcff7433a8c130ffd12ad Author: Nick Mathewson nickm@torproject.org Date: Tue Nov 29 17:06:09 2011 -0500

Make pending libevent actions cancelable

This avoids a dangling pointer issue in the 3412 code, and should fix bug 4599. --- src/common/compat_libevent.c | 24 +++++++++++++++++------- src/common/compat_libevent.h | 8 +++++--- src/common/tortls.c | 13 +++---------- src/or/connection_or.c | 25 +++++++++++++++++++++---- src/or/or.h | 2 ++ 5 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index 3a754be..67f4659 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -558,17 +558,17 @@ tor_check_libevent_header_compatibility(void) #endif }

-typedef struct runnable_t { +struct tor_libevent_action_t { struct event *ev; void (*cb)(void *arg); void *arg; -} runnable_t; +};

/** Callback for tor_run_in_libevent_loop */ static void run_runnable_cb(evutil_socket_t s, short what, void *arg) { - runnable_t *r = arg; + tor_libevent_action_t *r = arg; void (*cb)(void *) = r->cb; void *cb_arg = r->arg; (void)what; @@ -584,22 +584,32 @@ run_runnable_cb(evutil_socket_t s, short what, void *arg) * deep inside a no-reentrant code and there's some function you want to call * without worrying about whether it might cause reeentrant invocation. */ -int +tor_libevent_action_t * tor_run_in_libevent_loop(void (*cb)(void *arg), void *arg) { - runnable_t *r = tor_malloc(sizeof(runnable_t)); + tor_libevent_action_t *r = tor_malloc(sizeof(tor_libevent_action_t)); r->cb = cb; r->arg = arg; r->ev = tor_event_new(tor_libevent_get_base(), -1, EV_TIMEOUT, run_runnable_cb, r); if (!r->ev) { tor_free(r); - return -1; + return NULL; } /* Make the event active immediately. */ event_active(r->ev, EV_TIMEOUT, 1);

- return 0; + return r; +} + +/** + * Cancel <b>action</b> without running it. + */ +void +tor_cancel_libevent_action(tor_libevent_action_t *action) +{ + tor_event_free(action->ev); + tor_free(action); }

/* diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h index 3f916d1..4076cc0 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -44,10 +44,12 @@ void tor_event_free(struct event *ev); #define tor_evdns_add_server_port evdns_add_server_port #endif

-typedef struct periodic_timer_t periodic_timer_t; - -int tor_run_in_libevent_loop(void (*cb)(void *arg), void *arg); +typedef struct tor_libevent_action_t tor_libevent_action_t; +tor_libevent_action_t *tor_run_in_libevent_loop(void (*cb)(void *arg), + void *arg); +void tor_cancel_libevent_action(tor_libevent_action_t *action);

+typedef struct periodic_timer_t periodic_timer_t; periodic_timer_t *periodic_timer_new(struct event_base *base, const struct timeval *tv, void (*cb)(periodic_timer_t *timer, void *data), diff --git a/src/common/tortls.c b/src/common/tortls.c index b4d81de..a6947c8 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1339,16 +1339,9 @@ tor_tls_got_client_hello(tor_tls_t *tls) tls->excess_renegotiations_callback) { /* We got more than one renegotiation requests. The Tor protocol needs just one renegotiation; more than that probably means - They are trying to DoS us and we have to stop them. We can't - close their connection from in here since it's an OpenSSL - callback, so we set a libevent timer that triggers in the next - event loop and closes the connection. */ - - if (tor_run_in_libevent_loop(tls->excess_renegotiations_callback, - tls->callback_arg) < 0) { - log_warn(LD_GENERAL, "Didn't manage to set a renegotiation " - "limiting callback."); - } + They are trying to DoS us and we have to stop them. */ + + tls->excess_renegotiations_callback(tls->callback_arg); }

/* Now check the cipher list. */ diff --git a/src/or/connection_or.c b/src/or/connection_or.c index e868b7b..246b08a 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -492,6 +492,9 @@ connection_or_about_to_close(or_connection_t *or_conn) time_t now = time(NULL); connection_t *conn = TO_CONN(or_conn);

+ if (or_conn->pending_action) + tor_cancel_libevent_action(or_conn->pending_action); + /* Remember why we're closing this connection. */ if (conn->state != OR_CONN_STATE_OPEN) { /* Inform any pending (not attached) circs that they should @@ -1153,20 +1156,34 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn) } }

-/** Invoked on the server side using a timer from inside - * tor_tls_got_client_hello() when the server receives excess - * renegotiation attempts; probably indicating a DoS. */ +/*DOCDOC*/ static void -connection_or_close_connection_cb(void *_conn) +close_connection_libevent_cb(void *_conn) { or_connection_t *or_conn = _conn; connection_t *conn = TO_CONN(or_conn);

+ or_conn->pending_action = NULL; + connection_stop_reading(conn); if (!conn->marked_for_close) connection_mark_for_close(conn); }

+/* DOCDOC */ +static void +connection_or_close_connection_cb(void *_conn) +{ + /* We can't close their connection from in here since it's an OpenSSL + callback, so we set a libevent event that triggers in the next event + loop and closes the connection. */ + or_connection_t *or_conn = _conn; + if (or_conn->_base.marked_for_close || or_conn->pending_action) + return; + or_conn->pending_action = + tor_run_in_libevent_loop(close_connection_libevent_cb, or_conn); +} + /** Move forward with the tls handshake. If it finishes, hand * <b>conn</b> to connection_tls_finish_handshake(). * diff --git a/src/or/or.h b/src/or/or.h index 546fe17..185d20d 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1272,6 +1272,8 @@ typedef struct or_connection_t { unsigned active_circuit_pqueue_last_recalibrated; struct or_connection_t *next_with_same_id; /**< Next connection with same * identity digest as this one. */ + + tor_libevent_action_t *pending_action; } or_connection_t;

/** Subtype of connection_t for an "edge connection" -- that is, an entry (ap)