summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorErik Kline <ek@google.com>2014-09-30 16:06:37 +0900
committerErik Kline <ek@google.com>2014-09-30 18:46:23 +0900
commite226badb960695447af4c480bb183ec35c411bbf (patch)
tree3408a161e6f4093bb2f32367ae9202bc5b505a4b
parent68eff53e7ed9df06f194478930f39b31c7a32458 (diff)
downloadandroid_external_dnsmasq-e226badb960695447af4c480bb183ec35c411bbf.tar.gz
android_external_dnsmasq-e226badb960695447af4c480bb183ec35c411bbf.tar.bz2
android_external_dnsmasq-e226badb960695447af4c480bb183ec35c411bbf.zip
Fixup existing listeners struct irec pointers.
This fixes a difficult to diagnose bug in which when closing old listeners and creating new listeners, any listeners which stick around are regrettably left with their iface pointer pointing to a struct irec that is free()d at the end of the set_interfaces() call. This results in a situation where subsequent malloc()s can reuse this memory which, when written to, corrupts the listener's concept of its listening address (by overwriting iface.addr). This mean that when this listener is later closed because, say, tethering on its interface has been removed, the close logic is comparing IPv4 socket addresses with possible garbage, resulting in the socket not being closed because no matching listening address is found. Because the socket is never closed, if the interface is later re-added the bind() to the interface address fails with EADDRINUSE. Also: fix a bogus memset() invocation. Bug: 17475756 Change-Id: I369dcd50b1d03db279fdb2c1d7f0e048df21be65
-rwxr-xr-xsrc/network.c34
1 files changed, 31 insertions, 3 deletions
diff --git a/src/network.c b/src/network.c
index 43b1573..f2ccf34 100755
--- a/src/network.c
+++ b/src/network.c
@@ -479,6 +479,31 @@ void create_bound_listener(struct listener **listeners, struct irec *iface)
}
/**
+ * If a listener has a struct irec pointer whose address matches the newly
+ * malloc()d struct irec's address, update its pointer to refer to this new
+ * struct irec instance.
+ *
+ * Otherwise, any listeners that are preserved across interface list changes
+ * will point at interface structures that are free()d at the end of
+ * set_interfaces(), and can get overwritten by subsequent memory allocations.
+ *
+ * See b/17475756 for further discussion.
+ */
+void fixup_possible_existing_listener(struct irec *new_iface) {
+ /* find the listener, if present */
+ struct listener *l;
+ for (l = daemon->listeners; l; l = l->next) {
+ struct irec *listener_iface = l->iface;
+ if (listener_iface) {
+ if (sockaddr_isequal(&listener_iface->addr, &new_iface->addr)) {
+ l->iface = new_iface;
+ return;
+ }
+ }
+ }
+}
+
+/**
* Close the sockets listening on the given interface
*
* This new function is needed as we're dynamically changing the interfaces
@@ -922,7 +947,7 @@ void set_interfaces(const char *interfaces)
strncpy(s, interfaces, sizeof(s));
while((interface = strsep(&next, ":"))) {
if_tmp = safe_malloc(sizeof(struct iname));
- memset(if_tmp, sizeof(struct iname), 0);
+ memset(if_tmp, 0, sizeof(struct iname));
if ((if_tmp->name = strdup(interface)) == NULL) {
die(_("malloc failure in set_interfaces: %s"), NULL, EC_BADNET);
}
@@ -945,11 +970,14 @@ void set_interfaces(const char *interfaces)
int found = 0;
for (new_iface = daemon->interfaces; new_iface; new_iface = new_iface->next) {
if (sockaddr_isequal(&old_iface->addr, &new_iface->addr)) {
- found = -1;
+ found = 1;
break;
}
}
- if (!found) {
+
+ if (found) {
+ fixup_possible_existing_listener(new_iface);
+ } else {
#ifdef __ANDROID_DEBUG__
char debug_buff[MAXDNAME];
prettyprint_addr(&old_iface->addr, debug_buff);