From 31d6659d974800e972af43856405e8a7abe08f72 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 5 Jan 2011 16:02:43 -0500 Subject: [PATCH 1/3] Fix a double-counting bug in addrmap_get_virtual_address We were decrementing "available" twice for each in-use address we ran across. This would make us declare that we ran out of virtual addresses when the address space was only half full. --- changes/bug2328 | 4 ++++ src/or/connection_edge.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changes/bug2328 diff --git a/changes/bug2328 b/changes/bug2328 new file mode 100644 index 0000000000..e5ce492a82 --- /dev/null +++ b/changes/bug2328 @@ -0,0 +1,4 @@ + o Minor bugfixes + - Fix a bug where we would declare that we had run out of virtual + addresses when the address space was only half-exhausted. Bugfix + on 0.1.1.19-rc. diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 9ecdf5c7a9..6c23775315 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1178,7 +1178,7 @@ addressmap_get_virtual_address(int type) ++next_virtual_addr; --available; log_info(LD_CONFIG, "%d addrs available", (int)available); - if (! --available) { + if (! available) { log_warn(LD_CONFIG, "Ran out of virtual addresses!"); return NULL; } From eabddd8ca003af2788832208e9ab666f7d3e9378 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 5 Jan 2011 16:36:48 -0500 Subject: [PATCH 2/3] Handle a NULL return from addressmap_get_virtual_address Fix for bug 2328; bugfix on 0.1.2.1-alpha; bug found by doorss. --- changes/bug2328 | 6 +++++- src/or/connection_edge.c | 24 ++++++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/changes/bug2328 b/changes/bug2328 index e5ce492a82..f1a4fa89c5 100644 --- a/changes/bug2328 +++ b/changes/bug2328 @@ -1,4 +1,8 @@ o Minor bugfixes - Fix a bug where we would declare that we had run out of virtual addresses when the address space was only half-exhausted. Bugfix - on 0.1.1.19-rc. + on 0.1.2.1-alpha. + - Correctly handle the case where AutomapHostsOnResolve is set but no + virtual addresses are available. Fixes bug2328, bugfix on + 0.1.2.1-alpha. Bug found by doorss. + diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 6c23775315..a01a6e38a1 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1142,6 +1142,8 @@ address_is_in_virtual_range(const char *address) /** Return a newly allocated string holding an address of type * (one of RESOLVED_TYPE_{IPV4|HOSTNAME}) that has not yet been mapped, * and that is very unlikely to be the address of any real host. + * + * May return NULL if we have run out of virtual addresses. */ static char * addressmap_get_virtual_address(int type) @@ -1199,14 +1201,15 @@ addressmap_get_virtual_address(int type) * allocated string. If another address of the same type is already * mapped to new_address, try to return a copy of that address. * - * The string in new_address may be freed, or inserted into a map - * as appropriate. + * The string in new_address may be freed or inserted into a map + * as appropriate. May return NULL if are out of virtual addresses. **/ const char * addressmap_register_virtual_address(int type, char *new_address) { char **addrp; virtaddress_entry_t *vent; + int vent_needs_to_be_added = 0; tor_assert(new_address); tor_assert(addressmap); @@ -1215,7 +1218,7 @@ addressmap_register_virtual_address(int type, char *new_address) vent = strmap_get(virtaddress_reversemap, new_address); if (!vent) { vent = tor_malloc_zero(sizeof(virtaddress_entry_t)); - strmap_set(virtaddress_reversemap, new_address, vent); + vent_needs_to_be_added = 1; } addrp = (type == RESOLVED_TYPE_IPV4) ? @@ -1225,6 +1228,7 @@ addressmap_register_virtual_address(int type, char *new_address) if (ent && ent->new_address && !strcasecmp(new_address, ent->new_address)) { tor_free(new_address); + tor_assert(!vent_needs_to_be_added); return tor_strdup(*addrp); } else log_warn(LD_BUG, @@ -1236,7 +1240,14 @@ addressmap_register_virtual_address(int type, char *new_address) tor_free(*addrp); *addrp = addressmap_get_virtual_address(type); + if (!*addrp) { + tor_free(vent); + tor_free(new_address); + return NULL; + } log_info(LD_APP, "Registering map from %s to %s", *addrp, new_address); + if (vent_needs_to_be_added) + strmap_set(virtaddress_reversemap, new_address, vent); addressmap_register(*addrp, new_address, 2, ADDRMAPSRC_CONTROLLER); #if 0 @@ -1415,7 +1426,12 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, const char *new_addr; new_addr = addressmap_register_virtual_address( RESOLVED_TYPE_IPV4, tor_strdup(socks->address)); - tor_assert(new_addr); + if (! new_addr) { + log_warn(LD_APP, "Unable to automap address %s", + escaped_safe_str(socks->address)); + connection_mark_unattached_ap(conn, END_STREAM_REASON_INTERNAL); + return -1; + } log_info(LD_APP, "Automapping %s to %s", escaped_safe_str(socks->address), safe_str(new_addr)); strlcpy(socks->address, new_addr, sizeof(socks->address)); From 2008728df70969d1868b204d4d1059541229d66f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 6 Jan 2011 13:29:36 -0500 Subject: [PATCH 3/3] Notice a little faster if we're running out of virtual addresses We were not decrementing "available" every time we did ++next_virtual_addr in addressmap_get_virtual_address: we left out the --available when we skipped .00 and .255 addresses. This didn't actually cause a bug in most cases, since the failure mode was to keep looping around the virtual addresses until we found one, or until available hit zero. It could have given you an infinite loop rather than a useful message, however, if you said "VirtualAddrNetwork 127.0.0.255/32" or something broken like that. Spotted by cypherpunks --- src/or/connection_edge.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index a01a6e38a1..001408a79e 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1169,6 +1169,10 @@ addressmap_get_virtual_address(int type) while ((next_virtual_addr & 0xff) == 0 || (next_virtual_addr & 0xff) == 0xff) { ++next_virtual_addr; + if (! --available) { + log_warn(LD_CONFIG, "Ran out of virtual addresses!"); + return NULL; + } } in.s_addr = htonl(next_virtual_addr); tor_inet_ntoa(&in, buf, sizeof(buf));