2
0

Compare commits

..

2 Commits

Author SHA1 Message Date
42d554bd3e [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
2023-01-07 15:49:46 +01:00
6791ccbaec [angel] fix handling of notifications on worker stop
* fix handling of "simple calls" (notifications)
  when plugin/action can't be found (must not send
  an error response, as id == -1)
* server stop clears all plugins; don't log error in
  that stage

Change-Id: If5167a3bd6d069c4cfc6dad13e205ce18b724509
2023-01-07 15:49:46 +01:00
3 changed files with 35 additions and 26 deletions

View File

@ -502,19 +502,20 @@ static void listen_ref_release(liServer *srv, liInstance *i, liPlugin *p, liInst
if (g_atomic_int_dec_and_test(&sock->refcount)) { if (g_atomic_int_dec_and_test(&sock->refcount)) {
liPluginCoreConfig *config = (liPluginCoreConfig*) p->data; 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); g_hash_table_remove(config->listen_sockets, &sock->addr);
}
g_slice_free(listen_ref_resource, ref);
}
static void _listen_socket_free(gpointer ptr) {
listen_socket *sock = ptr;
li_sockaddr_clear(&sock->addr); li_sockaddr_clear(&sock->addr);
close(sock->fd); close(sock->fd);
g_slice_free(listen_socket, sock); g_slice_free(listen_socket, sock);
}
g_slice_free(listen_ref_resource, ref);
} }
static void listen_socket_add(liInstance *i, liPlugin *p, listen_socket *sock) { static void listen_socket_add(liInstance *i, liPlugin *p, listen_socket *sock) {
@ -971,7 +972,7 @@ static gboolean core_init(liServer *srv, liPlugin *p) {
p->handle_instance_replaced = core_instance_replaced; p->handle_instance_replaced = core_instance_replaced;
core_parse_init(srv, p); 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(); config->listen_masks = g_ptr_array_new();
li_angel_plugin_add_angel_cb(p, "listen", core_listen); li_angel_plugin_add_angel_cb(p, "listen", core_listen);

View File

@ -61,34 +61,41 @@ static void instance_angel_call_cb(liAngelConnection *acon,
liPlugins *ps = &srv->plugins; liPlugins *ps = &srv->plugins;
liPlugin *p; liPlugin *p;
liPluginHandleCallCB cb; liPluginHandleCallCB cb;
GString *errstr = NULL;
UNUSED(mod_len); UNUSED(mod_len);
UNUSED(action_len); UNUSED(action_len);
p = g_hash_table_lookup(ps->ht_plugins, mod); p = g_hash_table_lookup(ps->ht_plugins, mod);
if (!p) { if (!p) {
GString *errstr = g_string_sized_new(0); errstr = g_string_sized_new(0);
GError *err = NULL; g_string_printf(errstr, "Plugin '%s' not available in lighttpd-angel (action '%s')", mod, action);
g_string_printf(errstr, "Plugin '%s' not available in lighttpd-angel", mod); goto failed;
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;
} }
cb = (liPluginHandleCallCB)(intptr_t) g_hash_table_lookup(p->angel_callbacks, action); cb = (liPluginHandleCallCB)(intptr_t) g_hash_table_lookup(p->angel_callbacks, action);
if (!cb) { if (!cb) {
GString *errstr = g_string_sized_new(0); errstr = g_string_sized_new(0);
GError *err = NULL;
g_string_printf(errstr, "Action '%s' not available in plugin '%s' of lighttpd-angel", action, mod); 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)) { goto failed;
ERROR(srv, "Couldn't send result: %s", err->message);
g_error_free(err);
}
return;
} }
cb(srv, p, i, id, data); 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) { static void instance_angel_close_cb(liAngelConnection *acon, GError *err) {

View File

@ -191,7 +191,7 @@ static gboolean angel_dispatch(liAngelConnection *acon, GError **err) {
if (!li_idlist_is_used(acon->call_id_list, id)) { if (!li_idlist_is_used(acon->call_id_list, id)) {
g_mutex_unlock(acon->mutex); g_mutex_unlock(acon->mutex);
g_set_error(err, LI_ANGEL_CONNECTION_ERROR, LI_ANGEL_CONNECTION_INVALID_DATA, 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); close_fd_array(acon->parse.fds);
return FALSE; 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, mod_len, err)) return FALSE;
if (!li_angel_data_write_int32(buf, action_len, err)) return FALSE; if (!li_angel_data_write_int32(buf, action_len, err)) return FALSE;
} else { } 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;
if (!li_angel_data_write_int32(buf, 0, err)) return FALSE; if (!li_angel_data_write_int32(buf, 0, err)) return FALSE;
} }