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
