Property-related D-Bus calls may be pending replies when the related GSupplicantInterface structure is cleaned up. This results in an attempt to access already freed memory when calls complete.
Fixed by keeping a list of pending property calls, and canceling the calls related to an interface when the interface is removed. This is a similar change to the one made earlier for pending method calls. --- gsupplicant/dbus.c | 140 ++++++++++++++++++++++++++++++----------------- gsupplicant/dbus.h | 12 ++-- gsupplicant/supplicant.c | 33 ++++++----- 3 files changed, 116 insertions(+), 69 deletions(-) diff --git a/gsupplicant/dbus.c b/gsupplicant/dbus.c index 5b15bcd..130306e 100644 --- a/gsupplicant/dbus.c +++ b/gsupplicant/dbus.c @@ -59,10 +59,35 @@ static int find_method_call_by_caller(gconstpointer a, gconstpointer b) return method_call->caller != caller; } +static GSList *property_calls; + +struct property_call_data { + gpointer caller; + DBusPendingCall *pending_call; + supplicant_dbus_property_function function; + void *user_data; +}; + +static void property_call_free(void *pointer) +{ + struct property_call_data *property_call = pointer; + property_calls = g_slist_remove(property_calls, property_call); + g_free(property_call); +} + +static int find_property_call_by_caller(gconstpointer a, gconstpointer b) +{ + const struct property_call_data *property_call = a; + gconstpointer caller = b; + + return property_call->caller != caller; +} + void supplicant_dbus_setup(DBusConnection *conn) { connection = conn; method_calls = NULL; + property_calls = NULL; } void supplicant_dbus_array_foreach(DBusMessageIter *iter, @@ -124,14 +149,27 @@ void supplicant_dbus_property_foreach(DBusMessageIter *iter, } } -struct property_get_data { - supplicant_dbus_property_function function; - void *user_data; -}; +void supplicant_dbus_property_call_cancel_all(gpointer caller) +{ + while (property_calls) { + struct property_call_data *property_call; + GSList *elem = g_slist_find_custom(property_calls, caller, + find_property_call_by_caller); + if (!elem) + break; + + property_call = elem->data; + property_calls = g_slist_delete_link(property_calls, elem); + + dbus_pending_call_cancel(property_call->pending_call); + + dbus_pending_call_unref(property_call->pending_call); + } +} static void property_get_all_reply(DBusPendingCall *call, void *user_data) { - struct property_get_data *data = user_data; + struct property_call_data *property_call = user_data; DBusMessage *reply; DBusMessageIter iter; @@ -143,11 +181,11 @@ static void property_get_all_reply(DBusPendingCall *call, void *user_data) if (!dbus_message_iter_init(reply, &iter)) goto done; - supplicant_dbus_property_foreach(&iter, data->function, - data->user_data); + supplicant_dbus_property_foreach(&iter, property_call->function, + property_call->user_data); - if (data->function) - data->function(NULL, NULL, data->user_data); + if (property_call->function) + property_call->function(NULL, NULL, property_call->user_data); done: dbus_message_unref(reply); @@ -157,9 +195,9 @@ done: int supplicant_dbus_property_get_all(const char *path, const char *interface, supplicant_dbus_property_function function, - void *user_data) + void *user_data, gpointer caller) { - struct property_get_data *data; + struct property_call_data *property_call = NULL; DBusMessage *message; DBusPendingCall *call; @@ -169,14 +207,14 @@ int supplicant_dbus_property_get_all(const char *path, const char *interface, if (!path || !interface) return -EINVAL; - data = dbus_malloc0(sizeof(*data)); - if (!data) + property_call = g_try_new0(struct property_call_data, 1); + if (!property_call) return -ENOMEM; message = dbus_message_new_method_call(SUPPLICANT_SERVICE, path, DBUS_INTERFACE_PROPERTIES, "GetAll"); if (!message) { - dbus_free(data); + g_free(property_call); return -ENOMEM; } @@ -187,21 +225,23 @@ int supplicant_dbus_property_get_all(const char *path, const char *interface, if (!dbus_connection_send_with_reply(connection, message, &call, TIMEOUT)) { dbus_message_unref(message); - dbus_free(data); + g_free(property_call); return -EIO; } if (!call) { dbus_message_unref(message); - dbus_free(data); + g_free(property_call); return -EIO; } - data->function = function; - data->user_data = user_data; + property_call->caller = caller; + property_call->pending_call = call; + property_call->function = function; + property_call->user_data = user_data; dbus_pending_call_set_notify(call, property_get_all_reply, - data, dbus_free); + property_call, property_call_free); dbus_message_unref(message); @@ -210,7 +250,7 @@ int supplicant_dbus_property_get_all(const char *path, const char *interface, static void property_get_reply(DBusPendingCall *call, void *user_data) { - struct property_get_data *data = user_data; + struct property_call_data *property_call = user_data; DBusMessage *reply; DBusMessageIter iter; @@ -227,8 +267,9 @@ static void property_get_reply(DBusPendingCall *call, void *user_data) dbus_message_iter_recurse(&iter, &variant); - if (data->function) - data->function(NULL, &variant, data->user_data); + if (property_call->function) + property_call->function(NULL, &variant, + property_call->user_data); } done: dbus_message_unref(reply); @@ -239,9 +280,9 @@ done: int supplicant_dbus_property_get(const char *path, const char *interface, const char *method, supplicant_dbus_property_function function, - void *user_data) + void *user_data, gpointer caller) { - struct property_get_data *data; + struct property_call_data *property_call = NULL; DBusMessage *message; DBusPendingCall *call; @@ -251,15 +292,15 @@ int supplicant_dbus_property_get(const char *path, const char *interface, if (!path || !interface || !method) return -EINVAL; - data = dbus_malloc0(sizeof(*data)); - if (!data) + property_call = g_try_new0(struct property_call_data, 1); + if (!property_call) return -ENOMEM; message = dbus_message_new_method_call(SUPPLICANT_SERVICE, path, DBUS_INTERFACE_PROPERTIES, "Get"); if (!message) { - dbus_free(data); + g_free(property_call); return -ENOMEM; } @@ -271,35 +312,32 @@ int supplicant_dbus_property_get(const char *path, const char *interface, if (!dbus_connection_send_with_reply(connection, message, &call, TIMEOUT)) { dbus_message_unref(message); - dbus_free(data); + g_free(property_call); return -EIO; } if (!call) { dbus_message_unref(message); - dbus_free(data); + g_free(property_call); return -EIO; } - data->function = function; - data->user_data = user_data; + property_call->caller = caller; + property_call->pending_call = call; + property_call->function = function; + property_call->user_data = user_data; dbus_pending_call_set_notify(call, property_get_reply, - data, dbus_free); + property_call, property_call_free); dbus_message_unref(message); return 0; } -struct property_set_data { - supplicant_dbus_result_function function; - void *user_data; -}; - static void property_set_reply(DBusPendingCall *call, void *user_data) { - struct property_set_data *data = user_data; + struct property_call_data *property_call = user_data; DBusMessage *reply; DBusMessageIter iter; const char *error; @@ -313,8 +351,8 @@ static void property_set_reply(DBusPendingCall *call, void *user_data) dbus_message_iter_init(reply, &iter); - if (data->function) - data->function(error, &iter, data->user_data); + if (property_call->function) + property_call->function(error, &iter, property_call->user_data); dbus_message_unref(reply); @@ -325,9 +363,9 @@ int supplicant_dbus_property_set(const char *path, const char *interface, const char *key, const char *signature, supplicant_dbus_setup_function setup, supplicant_dbus_result_function function, - void *user_data) + void *user_data, gpointer caller) { - struct property_set_data *data; + struct property_call_data *property_call = NULL; DBusMessage *message; DBusMessageIter iter, value; DBusPendingCall *call; @@ -341,14 +379,14 @@ int supplicant_dbus_property_set(const char *path, const char *interface, if (!key || !signature || !setup) return -EINVAL; - data = dbus_malloc0(sizeof(*data)); - if (!data) + property_call = g_try_new0(struct property_call_data, 1); + if (!property_call) return -ENOMEM; message = dbus_message_new_method_call(SUPPLICANT_SERVICE, path, DBUS_INTERFACE_PROPERTIES, "Set"); if (!message) { - dbus_free(data); + g_free(property_call); return -ENOMEM; } @@ -366,21 +404,23 @@ int supplicant_dbus_property_set(const char *path, const char *interface, if (!dbus_connection_send_with_reply(connection, message, &call, TIMEOUT)) { dbus_message_unref(message); - dbus_free(data); + g_free(property_call); return -EIO; } if (!call) { dbus_message_unref(message); - dbus_free(data); + g_free(property_call); return -EIO; } - data->function = function; - data->user_data = user_data; + property_call->caller = caller; + property_call->pending_call = call; + property_call->function = function; + property_call->user_data = user_data; dbus_pending_call_set_notify(call, property_set_reply, - data, dbus_free); + property_call, property_call_free); dbus_message_unref(message); diff --git a/gsupplicant/dbus.h b/gsupplicant/dbus.h index 0117a1c..3a90406 100644 --- a/gsupplicant/dbus.h +++ b/gsupplicant/dbus.h @@ -54,27 +54,29 @@ void supplicant_dbus_property_foreach(DBusMessageIter *iter, int supplicant_dbus_property_get_all(const char *path, const char *interface, supplicant_dbus_property_function function, - void *user_data); + void *user_data, gpointer caller); int supplicant_dbus_property_get(const char *path, const char *interface, const char *method, supplicant_dbus_property_function function, - void *user_data); + void *user_data, gpointer caller); int supplicant_dbus_property_set(const char *path, const char *interface, const char *key, const char *signature, supplicant_dbus_setup_function setup, supplicant_dbus_result_function function, - void *user_data); + void *user_data, gpointer caller); + +void supplicant_dbus_property_call_cancel_all(gpointer caller); int supplicant_dbus_method_call(const char *path, const char *interface, const char *method, supplicant_dbus_setup_function setup, supplicant_dbus_result_function function, void *user_data, - void *caller); + gpointer caller); -void supplicant_dbus_method_call_cancel_all(void *caller); +void supplicant_dbus_method_call_cancel_all(gpointer caller); void supplicant_dbus_property_append_basic(DBusMessageIter *iter, const char *key, int type, void *val); diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index 0b42ce8..7b0f4d4 100644 --- a/gsupplicant/supplicant.c +++ b/gsupplicant/supplicant.c @@ -733,7 +733,7 @@ int g_supplicant_interface_set_apscan(GSupplicantInterface *interface, return supplicant_dbus_property_set(interface->path, SUPPLICANT_INTERFACE ".Interface", "ApScan", DBUS_TYPE_UINT32_AS_STRING, - set_apscan, NULL, &ap_scan); + set_apscan, NULL, &ap_scan, NULL); } void g_supplicant_interface_set_data(GSupplicantInterface *interface, @@ -850,7 +850,7 @@ int g_supplicant_interface_enable_selected_network(GSupplicantInterface *interfa return supplicant_dbus_property_set(interface->network_path, SUPPLICANT_INTERFACE ".Network", "Enabled", DBUS_TYPE_BOOLEAN_AS_STRING, - set_network_enabled, NULL, &enable); + set_network_enabled, NULL, &enable, NULL); } dbus_bool_t g_supplicant_interface_get_ready(GSupplicantInterface *interface) @@ -1125,7 +1125,7 @@ static void interface_network_added(DBusMessageIter *iter, void *user_data) supplicant_dbus_property_get_all(path, SUPPLICANT_INTERFACE ".Network", - network_property, network); + network_property, network, NULL); } static void interface_network_removed(DBusMessageIter *iter, void *user_data) @@ -1695,7 +1695,7 @@ static void interface_bss_added_without_keys(DBusMessageIter *iter, supplicant_dbus_property_get_all(bss->path, SUPPLICANT_INTERFACE ".BSS", - bss_property, bss); + bss_property, bss, NULL); bss_compute_security(bss); add_or_replace_bss_to_network(bss); @@ -1976,7 +1976,8 @@ static void interface_added(DBusMessageIter *iter, void *user_data) supplicant_dbus_property_get_all(path, SUPPLICANT_INTERFACE ".Interface", - interface_property, interface); + interface_property, interface, + interface); } static void interface_removed(DBusMessageIter *iter, void *user_data) @@ -1991,6 +1992,7 @@ static void interface_removed(DBusMessageIter *iter, void *user_data) interface = g_hash_table_lookup(interface_table, path); SUPPLICANT_DBG("Cancelling any pending DBus calls"); supplicant_dbus_method_call_cancel_all(interface); + supplicant_dbus_property_call_cancel_all(interface); g_hash_table_remove(interface_table, path); } @@ -2085,8 +2087,8 @@ static void signal_name_owner_changed(const char *path, DBusMessageIter *iter) if (strlen(new) > 0 && strlen(old) == 0) { system_available = TRUE; supplicant_dbus_property_get_all(SUPPLICANT_PATH, - SUPPLICANT_INTERFACE, - service_property, NULL); + SUPPLICANT_INTERFACE, + service_property, NULL, NULL); } } @@ -2163,7 +2165,7 @@ static void signal_scan_done(const char *path, DBusMessageIter *iter) } supplicant_dbus_property_get(path, SUPPLICANT_INTERFACE ".Interface", - "BSSs", scan_bss_data, interface); + "BSSs", scan_bss_data, interface, interface); } static void signal_bss_added(const char *path, DBusMessageIter *iter) @@ -2494,7 +2496,8 @@ static void signal_peer_found(const char *path, DBusMessageIter *iter) } supplicant_dbus_property_get_all(obj_path, - SUPPLICANT_INTERFACE ".Peer", peer_property, peer); + SUPPLICANT_INTERFACE ".Peer", + peer_property, peer, NULL); } static void signal_peer_lost(const char *path, DBusMessageIter *iter) @@ -2638,7 +2641,7 @@ int g_supplicant_set_country(const char *alpha2, return supplicant_dbus_property_set(SUPPLICANT_PATH, SUPPLICANT_INTERFACE, "Country", DBUS_TYPE_STRING_AS_STRING, country_params, country_result, - regdom); + regdom, NULL); } int g_supplicant_interface_set_country(GSupplicantInterface *interface, @@ -2660,7 +2663,7 @@ int g_supplicant_interface_set_country(GSupplicantInterface *interface, SUPPLICANT_INTERFACE ".Interface", "Country", DBUS_TYPE_STRING_AS_STRING, country_params, country_result, - regdom); + regdom, NULL); } bool g_supplicant_interface_has_p2p(GSupplicantInterface *interface) @@ -2783,7 +2786,8 @@ static void interface_create_result(const char *error, err = supplicant_dbus_property_get_all(path, SUPPLICANT_INTERFACE ".Interface", - interface_create_property, data); + interface_create_property, data, + data->interface); if (err == 0) return; @@ -2978,6 +2982,7 @@ int g_supplicant_interface_remove(GSupplicantInterface *interface, SUPPLICANT_DBG("Cancelling any pending DBus calls"); supplicant_dbus_method_call_cancel_all(interface); + supplicant_dbus_property_call_cancel_all(interface); data = dbus_malloc0(sizeof(*data)); if (!data) @@ -3908,7 +3913,7 @@ int g_supplicant_interface_connect(GSupplicantInterface *interface, ret = supplicant_dbus_property_set(interface->path, SUPPLICANT_INTERFACE ".Interface.WPS", "ProcessCredentials", DBUS_TYPE_BOOLEAN_AS_STRING, - wps_process_credentials, wps_start, data); + wps_process_credentials, wps_start, data, interface); } else ret = supplicant_dbus_method_call(interface->path, SUPPLICANT_INTERFACE ".Interface", "AddNetwork", @@ -4203,7 +4208,7 @@ int g_supplicant_register(const GSupplicantCallbacks *callbacks) system_available = TRUE; supplicant_dbus_property_get_all(SUPPLICANT_PATH, SUPPLICANT_INTERFACE, - service_property, NULL); + service_property, NULL, NULL); } else invoke_introspect_method(); -- 1.8.5.3 _______________________________________________ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman