From ed0ee340d4a9033524d28edbcecc648432525052 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Wed, 25 Apr 2018 14:32:27 +0200 Subject: Refactoring: Move code that creates listener for port into new function --- src/core/mainloop/connection.c | 103 +++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 44 deletions(-) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index ed789d5208..bb4130640b 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -1487,6 +1487,60 @@ connection_listener_new(const struct sockaddr *listensockaddr, return NULL; } +static connection_t * +connection_listener_new_for_port(const port_cfg_t *port, int *defer) +{ + connection_t *conn; + struct sockaddr *listensockaddr; + socklen_t listensocklen = 0; + char *address=NULL; + int real_port = port->port == CFG_AUTO_PORT ? 0 : port->port; + tor_assert(real_port <= UINT16_MAX); + + if (defer) + *defer = 0; + + if (port->server_cfg.no_listen) { + *defer = 1; + return NULL; + } + +#ifndef _WIN32 + /* We don't need to be root to create a UNIX socket, so defer until after + * setuid. */ + const or_options_t *options = get_options(); + if (port->is_unix_addr && !geteuid() && (options->User) && + strcmp(options->User, "root")) { + *defer = 1; + return NULL; + } +#endif /* !defined(_WIN32) */ + + if (port->is_unix_addr) { + listensockaddr = (struct sockaddr *) + create_unix_sockaddr(port->unix_addr, + &address, &listensocklen); + } else { + listensockaddr = tor_malloc(sizeof(struct sockaddr_storage)); + listensocklen = tor_addr_to_sockaddr(&port->addr, + real_port, + listensockaddr, + sizeof(struct sockaddr_storage)); + address = tor_addr_to_str_dup(&port->addr); + } + + if (listensockaddr) { + conn = connection_listener_new(listensockaddr, listensocklen, + port->type, address, port); + tor_free(listensockaddr); + tor_free(address); + } else { + conn = NULL; + } + + return conn; +} + /** Do basic sanity checking on a newly received socket. Return 0 * if it looks ok, else return -1. * @@ -2676,52 +2730,13 @@ retry_listener_ports(smartlist_t *old_conns, /* Now open all the listeners that are configured but not opened. */ SMARTLIST_FOREACH_BEGIN(launch, const port_cfg_t *, port) { - struct sockaddr *listensockaddr; - socklen_t listensocklen = 0; - char *address=NULL; - connection_t *conn; - int real_port = port->port == CFG_AUTO_PORT ? 0 : port->port; - tor_assert(real_port <= UINT16_MAX); - if (port->server_cfg.no_listen) - continue; - -#ifndef _WIN32 - /* We don't need to be root to create a UNIX socket, so defer until after - * setuid. */ - const or_options_t *options = get_options(); - if (port->is_unix_addr && !geteuid() && (options->User) && - strcmp(options->User, "root")) - continue; -#endif /* !defined(_WIN32) */ + int skip = 0; + connection_t *conn = connection_listener_new_for_port(port, &skip); - if (port->is_unix_addr) { - listensockaddr = (struct sockaddr *) - create_unix_sockaddr(port->unix_addr, - &address, &listensocklen); - } else { - listensockaddr = tor_malloc(sizeof(struct sockaddr_storage)); - listensocklen = tor_addr_to_sockaddr(&port->addr, - real_port, - listensockaddr, - sizeof(struct sockaddr_storage)); - address = tor_addr_to_str_dup(&port->addr); - } - - if (listensockaddr) { - conn = connection_listener_new(listensockaddr, listensocklen, - port->type, address, port); - tor_free(listensockaddr); - tor_free(address); - } else { - conn = NULL; - } - - if (!conn) { + if (conn && new_conns) + smartlist_add(new_conns, conn); + else if (!skip) r = -1; - } else { - if (new_conns) - smartlist_add(new_conns, conn); - } } SMARTLIST_FOREACH_END(port); smartlist_free(launch); -- cgit v1.2.3-54-g00ecf From c99bb8b6ea2b533e8ed99e6a41d74fc3bbeddb94 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Wed, 25 Apr 2018 16:17:11 +0200 Subject: Try rebinding new listener after closing old one if first bind failed with EADDRINUSE --- src/core/mainloop/connection.c | 106 +++++++++++++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 14 deletions(-) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index bb4130640b..22fa3fb795 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -137,7 +137,11 @@ static connection_t *connection_listener_new( const struct sockaddr *listensockaddr, socklen_t listensocklen, int type, const char *address, - const port_cfg_t *portcfg); + const port_cfg_t *portcfg, + int *addr_in_use); +static connection_t *connection_listener_new_for_port( + const port_cfg_t *port, + int *defer, int *addr_in_use); static void connection_init(time_t now, connection_t *conn, int type, int socket_family); static int connection_handle_listener_read(connection_t *conn, int new_type); @@ -1188,7 +1192,8 @@ static connection_t * connection_listener_new(const struct sockaddr *listensockaddr, socklen_t socklen, int type, const char *address, - const port_cfg_t *port_cfg) + const port_cfg_t *port_cfg, + int *addr_in_use) { listener_connection_t *lis_conn; connection_t *conn = NULL; @@ -1204,6 +1209,8 @@ connection_listener_new(const struct sockaddr *listensockaddr, tor_addr_t addr; int exhaustion = 0; + if (addr_in_use) *addr_in_use = 0; + if (listensockaddr->sa_family == AF_INET || listensockaddr->sa_family == AF_INET6) { int is_stream = (type != CONN_TYPE_AP_DNS_LISTENER); @@ -1282,8 +1289,10 @@ connection_listener_new(const struct sockaddr *listensockaddr, if (bind(s,listensockaddr,socklen) < 0) { const char *helpfulhint = ""; int e = tor_socket_errno(s); - if (ERRNO_IS_EADDRINUSE(e)) + if (ERRNO_IS_EADDRINUSE(e)) { helpfulhint = ". Is Tor already running?"; + if (addr_in_use) *addr_in_use = 1; + } log_warn(LD_NET, "Could not bind to %s:%u: %s%s", address, usePort, tor_socket_strerror(e), helpfulhint); goto err; @@ -1487,8 +1496,15 @@ connection_listener_new(const struct sockaddr *listensockaddr, return NULL; } +/** + * Create a new listener connection for a given port. In case we + * for a reason that is not an error condition, set defer + * to true. If we cannot bind listening socket because address is already + * in use, set addr_in_use to true. + */ static connection_t * -connection_listener_new_for_port(const port_cfg_t *port, int *defer) +connection_listener_new_for_port(const port_cfg_t *port, + int *defer, int *addr_in_use) { connection_t *conn; struct sockaddr *listensockaddr; @@ -1531,7 +1547,8 @@ connection_listener_new_for_port(const port_cfg_t *port, int *defer) if (listensockaddr) { conn = connection_listener_new(listensockaddr, listensocklen, - port->type, address, port); + port->type, address, port, + addr_in_use); tor_free(listensockaddr); tor_free(address); } else { @@ -2651,16 +2668,26 @@ connection_read_proxy_handshake(connection_t *conn) return ret; } +struct replacement_s +{ + connection_t *old_conn; + port_cfg_t *new_port; +}; + /** Given a list of listener connections in old_conns, and list of * port_cfg_t entries in ports, open a new listener for every port in * ports that does not already have a listener in old_conns. * * Remove from old_conns every connection that has a corresponding * entry in ports. Add to new_conns new every connection we - * launch. + * launch. If we may need to perform socket rebind when creating new + * listener that replaces old one, create a replacement_s struct + * for affected pair and add it to replacements. For more + * information, see ticket #17873. * * If control_listeners_only is true, then we only open control - * listeners, and we do not remove any noncontrol listeners from old_conns. + * listeners, and we do not remove any noncontrol listeners from + * old_conns. * * Return 0 on success, -1 on failure. **/ @@ -2668,8 +2695,10 @@ static int retry_listener_ports(smartlist_t *old_conns, const smartlist_t *ports, smartlist_t *new_conns, + smartlist_t *replacements, int control_listeners_only) { + smartlist_t *launch = smartlist_new(); int r = 0; @@ -2705,16 +2734,31 @@ retry_listener_ports(smartlist_t *old_conns, break; } } else { - int port_matches; - if (wanted->port == CFG_AUTO_PORT) { - port_matches = 1; - } else { - port_matches = (wanted->port == conn->port); - } + /* Numeric values of old and new port match exactly. */ + const int port_matches_exact = (wanted->port == conn->port); + /* Ports match semantically - either their specific values + match exactly, or new port is 'auto'. + */ + const int port_matches = (wanted->port == CFG_AUTO_PORT || + port_matches_exact); + if (port_matches && tor_addr_eq(&wanted->addr, &conn->addr)) { found_port = wanted; break; } + const int may_need_rebind = + port_matches_exact && bool_neq(tor_addr_is_null(&wanted->addr), + tor_addr_is_null(&conn->addr)); + if (replacements && may_need_rebind) { + struct replacement_s *replacement = + tor_malloc(sizeof(struct replacement_s)); + + replacement->old_conn = conn; + replacement->new_port = (port_cfg_t *)wanted; + smartlist_add(replacements, replacement); + + SMARTLIST_DEL_CURRENT(launch, wanted); + } } } SMARTLIST_FOREACH_END(wanted); @@ -2731,7 +2775,7 @@ retry_listener_ports(smartlist_t *old_conns, /* Now open all the listeners that are configured but not opened. */ SMARTLIST_FOREACH_BEGIN(launch, const port_cfg_t *, port) { int skip = 0; - connection_t *conn = connection_listener_new_for_port(port, &skip); + connection_t *conn = connection_listener_new_for_port(port, &skip, NULL); if (conn && new_conns) smartlist_add(new_conns, conn); @@ -2759,6 +2803,7 @@ retry_all_listeners(smartlist_t *replaced_conns, smartlist_t *new_conns, int close_all_noncontrol) { smartlist_t *listeners = smartlist_new(); + smartlist_t *replacements = smartlist_new(); const or_options_t *options = get_options(); int retval = 0; const uint16_t old_or_port = router_get_advertised_or_port(options); @@ -2774,9 +2819,42 @@ retry_all_listeners(smartlist_t *replaced_conns, if (retry_listener_ports(listeners, get_configured_ports(), new_conns, + replacements, close_all_noncontrol) < 0) retval = -1; + SMARTLIST_FOREACH_BEGIN(replacements, struct replacement_s *, r) { + int addr_in_use = 0; + int skip = 0; + + tor_assert(r->new_port); + tor_assert(r->old_conn); + + connection_t *new_conn = + connection_listener_new_for_port(r->new_port, &skip, &addr_in_use); + connection_t *old_conn = r->old_conn; + + + if (skip) + continue; + + // XXX: replaced_conns + connection_close_immediate(r->old_conn); + connection_mark_for_close(r->old_conn); + + if (addr_in_use) { + new_conn = connection_listener_new_for_port(r->new_port, + &skip, &addr_in_use); + } + + tor_assert(new_conn); + + smartlist_add(new_conns, new_conn); + + tor_free(r); + SMARTLIST_DEL_CURRENT(replacements, r); + } SMARTLIST_FOREACH_END(r); + /* Any members that were still in 'listeners' don't correspond to * any configured port. Kill 'em. */ SMARTLIST_FOREACH_BEGIN(listeners, connection_t *, conn) { -- cgit v1.2.3-54-g00ecf From f04e0bd5d630a2ef3b618f3fe6f05d46358dfb65 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Wed, 25 Apr 2018 17:27:06 +0200 Subject: Refrain from compiling socket rebinding code on system that don't need it --- src/core/mainloop/connection.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 22fa3fb795..9725c3030a 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -133,6 +133,10 @@ #include "feature/nodelist/routerinfo_st.h" #include "core/or/socks_request_st.h" +#if defined(__linux__) || defined(_WIN32) +#define ENABLE_LISTENER_REBIND +#endif + static connection_t *connection_listener_new( const struct sockaddr *listensockaddr, socklen_t listensocklen, int type, @@ -1517,7 +1521,7 @@ connection_listener_new_for_port(const port_cfg_t *port, *defer = 0; if (port->server_cfg.no_listen) { - *defer = 1; + if (defer) *defer = 1; return NULL; } @@ -1527,7 +1531,7 @@ connection_listener_new_for_port(const port_cfg_t *port, const or_options_t *options = get_options(); if (port->is_unix_addr && !geteuid() && (options->User) && strcmp(options->User, "root")) { - *defer = 1; + if (defer) *defer = 1; return NULL; } #endif /* !defined(_WIN32) */ @@ -2698,6 +2702,9 @@ retry_listener_ports(smartlist_t *old_conns, smartlist_t *replacements, int control_listeners_only) { +#ifndef ENABLE_LISTENER_REBIND + (void)replacements; +#endif smartlist_t *launch = smartlist_new(); int r = 0; @@ -2746,6 +2753,7 @@ retry_listener_ports(smartlist_t *old_conns, found_port = wanted; break; } +#ifdef ENABLE_LISTENER_REBIND const int may_need_rebind = port_matches_exact && bool_neq(tor_addr_is_null(&wanted->addr), tor_addr_is_null(&conn->addr)); @@ -2758,7 +2766,9 @@ retry_listener_ports(smartlist_t *old_conns, smartlist_add(replacements, replacement); SMARTLIST_DEL_CURRENT(launch, wanted); + SMARTLIST_DEL_CURRENT(old_conns, conn); } +#endif } } SMARTLIST_FOREACH_END(wanted); @@ -2823,6 +2833,7 @@ retry_all_listeners(smartlist_t *replaced_conns, close_all_noncontrol) < 0) retval = -1; +#ifdef ENABLE_LISTENER_REBIND SMARTLIST_FOREACH_BEGIN(replacements, struct replacement_s *, r) { int addr_in_use = 0; int skip = 0; @@ -2854,6 +2865,7 @@ retry_all_listeners(smartlist_t *replaced_conns, tor_free(r); SMARTLIST_DEL_CURRENT(replacements, r); } SMARTLIST_FOREACH_END(r); +#endif /* Any members that were still in 'listeners' don't correspond to * any configured port. Kill 'em. */ -- cgit v1.2.3-54-g00ecf From 9f7ed1d04e7afd261898236a70f3cd02697c4a01 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Thu, 26 Apr 2018 11:18:13 +0200 Subject: Always close old listeners in retry_all_listeners --- src/app/config/config.c | 16 +--------------- src/core/mainloop/connection.c | 13 +++---------- src/core/mainloop/connection.h | 3 +-- src/core/mainloop/main.c | 2 +- 4 files changed, 6 insertions(+), 28 deletions(-) (limited to 'src/core') diff --git a/src/app/config/config.c b/src/app/config/config.c index 1b1889779d..0645bea993 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -1397,7 +1397,6 @@ static int options_act_reversible(const or_options_t *old_options, char **msg) { smartlist_t *new_listeners = smartlist_new(); - smartlist_t *replaced_listeners = smartlist_new(); or_options_t *options = get_options_mutable(); int running_tor = options->command == CMD_RUN_TOR; int set_conn_limit = 0; @@ -1481,8 +1480,7 @@ options_act_reversible(const or_options_t *old_options, char **msg) * shutting down. If networking is disabled, this will close all but the * control listeners, but disable those. */ if (!we_are_hibernating()) { - if (retry_all_listeners(replaced_listeners, new_listeners, - options->DisableNetwork) < 0) { + if (retry_all_listeners(new_listeners, options->DisableNetwork) < 0) { *msg = tor_strdup("Failed to bind one of the listener ports."); goto rollback; } @@ -1618,17 +1616,6 @@ options_act_reversible(const or_options_t *old_options, char **msg) "Overwrite the log afterwards.", badness); } - SMARTLIST_FOREACH(replaced_listeners, connection_t *, conn, - { - int marked = conn->marked_for_close; - log_notice(LD_NET, "Closing old %s on %s:%d", - conn_type_to_string(conn->type), conn->address, conn->port); - connection_close_immediate(conn); - if (!marked) { - connection_mark_for_close(conn); - } - }); - if (set_conn_limit) { /* * If we adjusted the conn limit, recompute the OOS threshold too @@ -1682,7 +1669,6 @@ options_act_reversible(const or_options_t *old_options, char **msg) done: smartlist_free(new_listeners); - smartlist_free(replaced_listeners); return r; } diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 9725c3030a..1cc83016fd 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -2802,15 +2802,13 @@ retry_listener_ports(smartlist_t *old_conns, * listeners who are not already open, and only close listeners we no longer * want. * - * Add all old conns that should be closed to replaced_conns. * Add all new connections to new_conns. * * If close_all_noncontrol is true, then we only open control * listeners, and we close all other listeners. */ int -retry_all_listeners(smartlist_t *replaced_conns, - smartlist_t *new_conns, int close_all_noncontrol) +retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol) { smartlist_t *listeners = smartlist_new(); smartlist_t *replacements = smartlist_new(); @@ -2849,7 +2847,6 @@ retry_all_listeners(smartlist_t *replaced_conns, if (skip) continue; - // XXX: replaced_conns connection_close_immediate(r->old_conn); connection_mark_for_close(r->old_conn); @@ -2872,12 +2869,8 @@ retry_all_listeners(smartlist_t *replaced_conns, SMARTLIST_FOREACH_BEGIN(listeners, connection_t *, conn) { log_notice(LD_NET, "Closing no-longer-configured %s on %s:%d", conn_type_to_string(conn->type), conn->address, conn->port); - if (replaced_conns) { - smartlist_add(replaced_conns, conn); - } else { - connection_close_immediate(conn); - connection_mark_for_close(conn); - } + connection_close_immediate(conn); + connection_mark_for_close(conn); } SMARTLIST_FOREACH_END(conn); smartlist_free(listeners); diff --git a/src/core/mainloop/connection.h b/src/core/mainloop/connection.h index 3419ee65e8..2552808fd8 100644 --- a/src/core/mainloop/connection.h +++ b/src/core/mainloop/connection.h @@ -178,8 +178,7 @@ void log_failed_proxy_connection(connection_t *conn); int get_proxy_addrport(tor_addr_t *addr, uint16_t *port, int *proxy_type, const connection_t *conn); -int retry_all_listeners(smartlist_t *replaced_conns, - smartlist_t *new_conns, +int retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol); void connection_mark_all_noncontrol_listeners(void); diff --git a/src/core/mainloop/main.c b/src/core/mainloop/main.c index c648d236bb..a50c852ecd 100644 --- a/src/core/mainloop/main.c +++ b/src/core/mainloop/main.c @@ -2338,7 +2338,7 @@ retry_listeners_callback(time_t now, const or_options_t *options) (void)now; (void)options; if (!net_is_disabled()) { - retry_all_listeners(NULL, NULL, 0); + retry_all_listeners(NULL, 0); return 60; } return PERIODIC_EVENT_NO_UPDATE; -- cgit v1.2.3-54-g00ecf From d548453abdf6ed708b6cb7e0a58f11d40f0cf4d1 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Thu, 26 Apr 2018 11:25:16 +0200 Subject: Log a notice when changing to/from wildcard IP address --- src/core/mainloop/connection.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 1cc83016fd..f435b29853 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -2843,12 +2843,11 @@ retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol) connection_listener_new_for_port(r->new_port, &skip, &addr_in_use); connection_t *old_conn = r->old_conn; - if (skip) continue; - connection_close_immediate(r->old_conn); - connection_mark_for_close(r->old_conn); + connection_close_immediate(old_conn); + connection_mark_for_close(old_conn); if (addr_in_use) { new_conn = connection_listener_new_for_port(r->new_port, @@ -2859,6 +2858,11 @@ retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol) smartlist_add(new_conns, new_conn); + log_notice(LD_NET, "Closed no-longer-configured %s on %s:%d " + "(replaced by %s:%d)", + conn_type_to_string(old_conn->type), old_conn->address, + old_conn->port, new_conn->address, new_conn->port); + tor_free(r); SMARTLIST_DEL_CURRENT(replacements, r); } SMARTLIST_FOREACH_END(r); -- cgit v1.2.3-54-g00ecf From 27c868eff19dbcc208f6db66ec3e2de2104fa754 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Sun, 20 May 2018 19:10:02 +0200 Subject: Log a notice *after* creating connection --- src/core/mainloop/connection.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index f435b29853..a32e3b4f27 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -1486,6 +1486,8 @@ connection_listener_new(const struct sockaddr *listensockaddr, */ connection_check_oos(get_n_open_sockets(), 0); + log_notice(LD_NET, "Opened %s on %s", + conn_type_to_string(type), fmt_addrport(&addr, usePort)); return conn; err: -- cgit v1.2.3-54-g00ecf From 74a474a2e78fdc977372cccfde37a2e92694f2df Mon Sep 17 00:00:00 2001 From: rl1987 Date: Mon, 7 May 2018 14:26:27 +0200 Subject: Minor code cleanups --- src/core/mainloop/connection.c | 30 ++++++++++++++---------------- src/core/mainloop/connection.h | 11 +++++++++++ 2 files changed, 25 insertions(+), 16 deletions(-) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index a32e3b4f27..200ea42433 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -1213,7 +1213,8 @@ connection_listener_new(const struct sockaddr *listensockaddr, tor_addr_t addr; int exhaustion = 0; - if (addr_in_use) *addr_in_use = 0; + if (addr_in_use) + *addr_in_use = 0; if (listensockaddr->sa_family == AF_INET || listensockaddr->sa_family == AF_INET6) { @@ -1295,7 +1296,8 @@ connection_listener_new(const struct sockaddr *listensockaddr, int e = tor_socket_errno(s); if (ERRNO_IS_EADDRINUSE(e)) { helpfulhint = ". Is Tor already running?"; - if (addr_in_use) *addr_in_use = 1; + if (addr_in_use) + *addr_in_use = 1; } log_warn(LD_NET, "Could not bind to %s:%u: %s%s", address, usePort, tor_socket_strerror(e), helpfulhint); @@ -1523,7 +1525,8 @@ connection_listener_new_for_port(const port_cfg_t *port, *defer = 0; if (port->server_cfg.no_listen) { - if (defer) *defer = 1; + if (defer) + *defer = 1; return NULL; } @@ -1533,7 +1536,8 @@ connection_listener_new_for_port(const port_cfg_t *port, const or_options_t *options = get_options(); if (port->is_unix_addr && !geteuid() && (options->User) && strcmp(options->User, "root")) { - if (defer) *defer = 1; + if (defer) + *defer = 1; return NULL; } #endif /* !defined(_WIN32) */ @@ -2674,12 +2678,6 @@ connection_read_proxy_handshake(connection_t *conn) return ret; } -struct replacement_s -{ - connection_t *old_conn; - port_cfg_t *new_port; -}; - /** Given a list of listener connections in old_conns, and list of * port_cfg_t entries in ports, open a new listener for every port in * ports that does not already have a listener in old_conns. @@ -2687,8 +2685,8 @@ struct replacement_s * Remove from old_conns every connection that has a corresponding * entry in ports. Add to new_conns new every connection we * launch. If we may need to perform socket rebind when creating new - * listener that replaces old one, create a replacement_s struct - * for affected pair and add it to replacements. For more + * listener that replaces old one, create a listener_replacement_t + * struct for affected pair and add it to replacements. For more * information, see ticket #17873. * * If control_listeners_only is true, then we only open control @@ -2760,11 +2758,11 @@ retry_listener_ports(smartlist_t *old_conns, port_matches_exact && bool_neq(tor_addr_is_null(&wanted->addr), tor_addr_is_null(&conn->addr)); if (replacements && may_need_rebind) { - struct replacement_s *replacement = - tor_malloc(sizeof(struct replacement_s)); + listener_replacement_t *replacement = + tor_malloc(sizeof(listener_replacement_t)); replacement->old_conn = conn; - replacement->new_port = (port_cfg_t *)wanted; + replacement->new_port = wanted; smartlist_add(replacements, replacement); SMARTLIST_DEL_CURRENT(launch, wanted); @@ -2834,7 +2832,7 @@ retry_all_listeners(smartlist_t *new_conns, int close_all_noncontrol) retval = -1; #ifdef ENABLE_LISTENER_REBIND - SMARTLIST_FOREACH_BEGIN(replacements, struct replacement_s *, r) { + SMARTLIST_FOREACH_BEGIN(replacements, listener_replacement_t *, r) { int addr_in_use = 0; int skip = 0; diff --git a/src/core/mainloop/connection.h b/src/core/mainloop/connection.h index 2552808fd8..b569bb038e 100644 --- a/src/core/mainloop/connection.h +++ b/src/core/mainloop/connection.h @@ -81,6 +81,17 @@ struct buf_t; /** State for any listener connection. */ #define LISTENER_STATE_READY 0 +/** + * This struct associates an old listener connection to be replaced + * by new connection described by port configuration. Only used when + * moving listeners to/from wildcard IP address. + */ +typedef struct +{ + connection_t *old_conn; /* Old listener connection to be replaced */ + const port_cfg_t *new_port; /* New port configuration */ +} listener_replacement_t; + const char *conn_type_to_string(int type); const char *conn_state_to_string(int type, int state); int conn_listener_type_supports_af_unix(int type); -- cgit v1.2.3-54-g00ecf From 9f5431c79fbe51af9445f00bb1a4d67d80e5ada5 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Wed, 16 May 2018 17:16:46 +0200 Subject: Comments/explanation for #17873 --- src/core/mainloop/connection.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 200ea42433..f1716ed8f0 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -133,6 +133,22 @@ #include "feature/nodelist/routerinfo_st.h" #include "core/or/socks_request_st.h" +/** + * On Windows and Linux we cannot reliably bind() a socket to an + * address and port if: 1) There's already a socket bound to wildcard + * address (0.0.0.0 or ::) with the same port; 2) We try to bind() + * to wildcard address and there's another socket bound to a + * specific address and the same port. + * + * To address this problem on these two platforms we implement a + * routine that: + * 1) Checks if first attempt to bind() a new socket failed with + * EADDRINUSE. + * 2) If so, it will close the appropriate old listener connection and + * 3) Attempts bind()'ing the new listener socket again. + * + * For further information, see ticket #17873. + */ #if defined(__linux__) || defined(_WIN32) #define ENABLE_LISTENER_REBIND #endif @@ -1191,6 +1207,9 @@ tor_listen(tor_socket_t fd) * * address is only used for logging purposes and to add the information * to the conn. + * + * Set addr_in_use to true in case socket binding fails with + * EADDRINUSE. */ static connection_t * connection_listener_new(const struct sockaddr *listensockaddr, -- cgit v1.2.3-54-g00ecf From fbd50f599408ae5d5e7bd7d8d63b23b9039524c7 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Wed, 16 May 2018 17:23:42 +0200 Subject: Avoid mentioning ticket number in comments --- src/core/mainloop/connection.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index f1716ed8f0..aeaff3784a 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -146,8 +146,6 @@ * EADDRINUSE. * 2) If so, it will close the appropriate old listener connection and * 3) Attempts bind()'ing the new listener socket again. - * - * For further information, see ticket #17873. */ #if defined(__linux__) || defined(_WIN32) #define ENABLE_LISTENER_REBIND @@ -2705,8 +2703,7 @@ connection_read_proxy_handshake(connection_t *conn) * entry in ports. Add to new_conns new every connection we * launch. If we may need to perform socket rebind when creating new * listener that replaces old one, create a listener_replacement_t - * struct for affected pair and add it to replacements. For more - * information, see ticket #17873. + * struct for affected pair and add it to replacements. * * If control_listeners_only is true, then we only open control * listeners, and we do not remove any noncontrol listeners from -- cgit v1.2.3-54-g00ecf From d8157097b42b6377d0cd83c758efce5825df83f2 Mon Sep 17 00:00:00 2001 From: rl1987 Date: Tue, 17 Jul 2018 13:04:06 +0000 Subject: Always include socket rebinding code --- src/core/mainloop/connection.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/core') diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index aeaff3784a..47a93e43a3 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -146,10 +146,11 @@ * EADDRINUSE. * 2) If so, it will close the appropriate old listener connection and * 3) Attempts bind()'ing the new listener socket again. + * + * Just to be safe, we are enabling listener rebind code on all platforms, + * to account for unexpected cases where it may be needed. */ -#if defined(__linux__) || defined(_WIN32) #define ENABLE_LISTENER_REBIND -#endif static connection_t *connection_listener_new( const struct sockaddr *listensockaddr, -- cgit v1.2.3-54-g00ecf