[angel] fix use-after-free of cached listening sockets

On stop the cached listenings sockets were freed through the hashtable;
but they are actually refcounted, and the hashtable only has a "weak"
reference (which gets cleared once all strong references are dropped).

Change-Id: I5f9c14f1c1e28f3a2b73955e69e36e1f9dae5780

src/angel/angel_plugin_core.c

@@ -502,21 +502,22 @@ static void listen_ref_release(liServer *srv, liInstance *i, liPlugin *p, liInst
 	if (g_atomic_int_dec_and_test(&sock->refcount)) {
 		liPluginCoreConfig *config = (liPluginCoreConfig*) p->data;
 
+		/* theoretically the hash table entry might not point to `sock`,
+		 * but a) that shouldn't happen (can't bind two sockets to same
+		 * address) and b) it doesn't matter - it just means the next
+		 * `core_listen` will try to bind a new one (and fail...).
+		 */
 		g_hash_table_remove(config->listen_sockets, &sock->addr);
+
+		li_sockaddr_clear(&sock->addr);
+		close(sock->fd);
+
+		g_slice_free(listen_socket, sock);
 	}
 
 	g_slice_free(listen_ref_resource, ref);
 }
 
-static void _listen_socket_free(gpointer ptr) {
-	listen_socket *sock = ptr;
-
-	li_sockaddr_clear(&sock->addr);
-	close(sock->fd);
-
-	g_slice_free(listen_socket, sock);
-}
-
 static void listen_socket_add(liInstance *i, liPlugin *p, listen_socket *sock) {
 	listen_ref_resource *ref = g_slice_new0(listen_ref_resource);
 
@@ -971,7 +972,7 @@ static gboolean core_init(liServer *srv, liPlugin *p) {
 	p->handle_instance_replaced = core_instance_replaced;
 
 	core_parse_init(srv, p);
-	config->listen_sockets = g_hash_table_new_full(li_hash_sockaddr, li_equal_sockaddr, NULL, _listen_socket_free);
+	config->listen_sockets = g_hash_table_new_full(li_hash_sockaddr, li_equal_sockaddr, NULL, NULL);
 	config->listen_masks = g_ptr_array_new();
 
 	li_angel_plugin_add_angel_cb(p, "listen", core_listen);

src/angel/angel_server.c

@@ -61,34 +61,41 @@ static void instance_angel_call_cb(liAngelConnection *acon,
 	liPlugins *ps = &srv->plugins;
 	liPlugin *p;
 	liPluginHandleCallCB cb;
+	GString *errstr = NULL;
 	UNUSED(mod_len);
 	UNUSED(action_len);
 
 	p = g_hash_table_lookup(ps->ht_plugins, mod);
 	if (!p) {
-		GString *errstr = g_string_sized_new(0);
-		GError *err = NULL;
-		g_string_printf(errstr, "Plugin '%s' not available in lighttpd-angel", mod);
-		if (!li_angel_send_result(acon, id, errstr, NULL, NULL, &err)) {
-			ERROR(srv, "Couldn't send result: %s", err->message);
-			g_error_free(err);
-		}
-		return;
+		errstr = g_string_sized_new(0);
+		g_string_printf(errstr, "Plugin '%s' not available in lighttpd-angel (action '%s')", mod, action);
+		goto failed;
 	}
 
 	cb = (liPluginHandleCallCB)(intptr_t) g_hash_table_lookup(p->angel_callbacks, action);
 	if (!cb) {
-		GString *errstr = g_string_sized_new(0);
-		GError *err = NULL;
+		errstr = g_string_sized_new(0);
 		g_string_printf(errstr, "Action '%s' not available in plugin '%s' of lighttpd-angel", action, mod);
-		if (!li_angel_send_result(acon, id, errstr, NULL, NULL, &err)) {
-			ERROR(srv, "Couldn't send result: %s", err->message);
-			g_error_free(err);
-		}
-		return;
+		goto failed;
 	}
 
 	cb(srv, p, i, id, data);
+	return;
+
+failed:
+	{
+		GError *err = NULL;
+		if (-1 == id) {
+			/* if there are no plugins the server was probably stopped - no need to log then */
+			if (0 != g_hash_table_size(ps->ht_plugins)) {
+				ERROR(srv, "Can't handle notification from worker: %s", errstr->str);
+			}
+			g_string_free(errstr, TRUE);
+		} else if (!li_angel_send_result(acon, id, errstr, NULL, NULL, &err)) {
+			ERROR(srv, "Couldn't send result: %s", err->message);
+			g_error_free(err);
+		}
+	}
 }
 
 static void instance_angel_close_cb(liAngelConnection *acon, GError *err) {

src/common/angel_connection.c

@@ -191,7 +191,7 @@ static gboolean angel_dispatch(liAngelConnection *acon, GError **err) {
 			if (!li_idlist_is_used(acon->call_id_list, id)) {
 				g_mutex_unlock(acon->mutex);
 				g_set_error(err, LI_ANGEL_CONNECTION_ERROR, LI_ANGEL_CONNECTION_INVALID_DATA,
-					"Invalid id: %i", (gint) id);
+					"Invalid id: %i (not waiting for the result of a call with this id)", (gint) id);
 				close_fd_array(acon->parse.fds);
 				return FALSE;
 			}
@@ -566,6 +566,7 @@ static gboolean prepare_call_header(GString **pbuf,
 		if (!li_angel_data_write_int32(buf, mod_len, err)) return FALSE;
 		if (!li_angel_data_write_int32(buf, action_len, err)) return FALSE;
 	} else {
+		LI_FORCE_ASSERT(id >= 0);
 		if (!li_angel_data_write_int32(buf, 0, err)) return FALSE;
 		if (!li_angel_data_write_int32(buf, 0, err)) return FALSE;
 	}