It is possible to overwrite the pending_call and pending_slot fields
in the GSupplicantInterface structure by making a new supplicant
related DBus call while an older call is still ongoing. This causes
problems with cleanup at interface removal time, as the older call is
not canceled and its reply processing may access already freed memory.
Fixed by having supplicant DBus code keep track of pending calls in a
list which is updated when call replies are processed. Supplicant code
can request cancellation of all pending calls for a particular
interface when needed.
---
gsupplicant/dbus.c | 98 +++++++++++++++++++++++++++++-------------------
gsupplicant/dbus.h | 6 +--
gsupplicant/supplicant.c | 82 +++++++++-------------------------------
3 files changed, 80 insertions(+), 106 deletions(-)
diff --git a/gsupplicant/dbus.c b/gsupplicant/dbus.c
index 27a132a..5b15bcd 100644
--- a/gsupplicant/dbus.c
+++ b/gsupplicant/dbus.c
@@ -35,9 +35,34 @@
static DBusConnection *connection;
+static GSList *method_calls;
+
+struct method_call_data {
+ gpointer caller;
+ DBusPendingCall *pending_call;
+ supplicant_dbus_result_function function;
+ void *user_data;
+};
+
+static void method_call_free(void *pointer)
+{
+ struct method_call_data *method_call = pointer;
+ method_calls = g_slist_remove(method_calls, method_call);
+ g_free(method_call);
+}
+
+static int find_method_call_by_caller(gconstpointer a, gconstpointer b)
+{
+ const struct method_call_data *method_call = a;
+ gconstpointer caller = b;
+
+ return method_call->caller != caller;
+}
+
void supplicant_dbus_setup(DBusConnection *conn)
{
connection = conn;
+ method_calls = NULL;
}
void supplicant_dbus_array_foreach(DBusMessageIter *iter,
@@ -362,27 +387,31 @@ int supplicant_dbus_property_set(const char *path, const
char *interface,
return 0;
}
-struct method_call_data {
- supplicant_dbus_result_function function;
- void *user_data;
-};
-
-void supplicant_dbus_call_callback(DBusPendingCall *call, dbus_int32_t slot)
+void supplicant_dbus_method_call_cancel_all(gpointer caller)
{
- struct method_call_data *data;
+ while (method_calls) {
+ struct method_call_data *method_call;
+ GSList *elem = g_slist_find_custom(method_calls, caller,
+ find_method_call_by_caller);
+ if (!elem)
+ break;
- data = dbus_pending_call_get_data(call, slot);
- if (data && data->function)
- data->function("net.connman.Error.OperationAborted",
- NULL, data->user_data);
+ method_call = elem->data;
+ method_calls = g_slist_delete_link(method_calls, elem);
- dbus_pending_call_free_data_slot(&slot);
- dbus_pending_call_unref(call);
+ dbus_pending_call_cancel(method_call->pending_call);
+
+ if (method_call->function)
+
method_call->function("net.connman.Error.OperationAborted",
+ NULL, method_call->user_data);
+
+ dbus_pending_call_unref(method_call->pending_call);
+ }
}
static void method_call_reply(DBusPendingCall *call, void *user_data)
{
- struct method_call_data *data;
+ struct method_call_data *method_call = user_data;
DBusMessage *reply;
DBusMessageIter iter;
const char *error;
@@ -396,9 +425,8 @@ static void method_call_reply(DBusPendingCall *call, void
*user_data)
dbus_message_iter_init(reply, &iter);
- data = dbus_pending_call_get_data(call, GPOINTER_TO_INT(user_data));
- if (data && data->function)
- data->function(error, &iter, data->user_data);
+ if (method_call && method_call->function)
+ method_call->function(error, &iter, method_call->user_data);
dbus_message_unref(reply);
@@ -410,14 +438,12 @@ int supplicant_dbus_method_call(const char *path,
supplicant_dbus_setup_function setup,
supplicant_dbus_result_function function,
void *user_data,
- DBusPendingCall **pending_call,
- dbus_int32_t *data_slot)
+ gpointer caller)
{
- struct method_call_data *data;
+ struct method_call_data *method_call = NULL;
DBusMessage *message;
DBusMessageIter iter;
DBusPendingCall *call;
- dbus_int32_t slot = -1;
if (!connection)
return -EINVAL;
@@ -425,14 +451,14 @@ int supplicant_dbus_method_call(const char *path,
if (!path || !interface || !method)
return -EINVAL;
- data = dbus_malloc0(sizeof(*data));
- if (!data)
+ method_call = g_try_new0(struct method_call_data, 1);
+ if (!method_call)
return -ENOMEM;
message = dbus_message_new_method_call(SUPPLICANT_SERVICE, path,
interface, method);
if (!message) {
- dbus_free(data);
+ g_free(method_call);
return -ENOMEM;
}
@@ -445,33 +471,27 @@ int supplicant_dbus_method_call(const char *path,
if (!dbus_connection_send_with_reply(connection, message,
&call, TIMEOUT)) {
dbus_message_unref(message);
- dbus_free(data);
+ g_free(method_call);
return -EIO;
}
if (!call) {
dbus_message_unref(message);
- dbus_free(data);
+ g_free(method_call);
return -EIO;
}
- data->function = function;
- data->user_data = user_data;
+ method_call->caller = caller;
+ method_call->pending_call = call;
+ method_call->function = function;
+ method_call->user_data = user_data;
+ method_calls = g_slist_prepend(method_calls, method_call);
- if (dbus_pending_call_allocate_data_slot(&slot)) {
- dbus_pending_call_set_data(call, slot, data, dbus_free);
- dbus_pending_call_set_notify(call, method_call_reply,
- GINT_TO_POINTER(slot), NULL);
- }
+ dbus_pending_call_set_notify(call, method_call_reply, method_call,
+ method_call_free);
dbus_message_unref(message);
- if (pending_call)
- *pending_call = call;
-
- if (data_slot)
- *data_slot = slot;
-
return 0;
}
diff --git a/gsupplicant/dbus.h b/gsupplicant/dbus.h
index 1f4c106..0117a1c 100644
--- a/gsupplicant/dbus.h
+++ b/gsupplicant/dbus.h
@@ -71,10 +71,10 @@ 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, DBusPendingCall **pending,
- dbus_int32_t *slot);
+ void *user_data,
+ void *caller);
-void supplicant_dbus_call_callback(DBusPendingCall *call, dbus_int32_t slot);
+void supplicant_dbus_method_call_cancel_all(void *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 b483f9b..8290f64 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -172,8 +172,6 @@ struct _GSupplicantInterface {
GHashTable *net_mapping;
GHashTable *bss_mapping;
void *data;
- DBusPendingCall *pending_call;
- dbus_int32_t pending_slot;
};
struct g_supplicant_bss {
@@ -1966,7 +1964,7 @@ static void interface_added(DBusMessageIter *iter, void
*user_data)
supplicant_dbus_method_call(path,
SUPPLICANT_INTERFACE ".Interface.P2PDevice", "Flush",
- NULL, interface_p2p_flush, interface, NULL, NULL);
+ NULL, interface_p2p_flush, interface, NULL);
dbus_message_iter_next(iter);
if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INVALID) {
@@ -1991,17 +1989,8 @@ static void interface_removed(DBusMessageIter *iter,
void *user_data)
return;
interface = g_hash_table_lookup(interface_table, path);
- if (interface && interface->pending_call) {
- dbus_pending_call_cancel(interface->pending_call);
- SUPPLICANT_DBG("Cancelled pending DBus call %p slot %d",
- interface->pending_call, interface->pending_slot);
-
- supplicant_dbus_call_callback(interface->pending_call,
- interface->pending_slot);
-
- interface->pending_call = NULL;
- interface->pending_slot = -1;
- }
+ SUPPLICANT_DBG("Cancelling any pending DBus calls");
+ supplicant_dbus_method_call_cancel_all(interface);
g_hash_table_remove(interface_table, path);
}
@@ -2847,8 +2836,6 @@ static void interface_get_result(const char *error,
goto done;
}
- interface->pending_call = NULL;
-
if (data->callback)
data->callback(0, interface, data->user_data);
@@ -2869,7 +2856,7 @@ create:
"CreateInterface",
interface_create_params,
interface_create_result, data,
- NULL, NULL);
+ NULL);
if (err == 0)
return;
@@ -2920,7 +2907,7 @@ int g_supplicant_interface_create(const char *ifname,
const char *driver,
"GetInterface",
interface_get_params,
interface_get_result, data,
- NULL, NULL);
+ NULL);
if (ret < 0)
dbus_free(data);
@@ -2950,9 +2937,6 @@ static void interface_remove_result(const char *error,
err = 0;
done:
- if (interface_exists(data->interface, data->path))
- data->interface->pending_call = NULL;
-
g_free(data->path);
if (data->callback)
@@ -2984,17 +2968,8 @@ int g_supplicant_interface_remove(GSupplicantInterface
*interface,
if (!system_available)
return -EFAULT;
- if (interface->pending_call) {
- dbus_pending_call_cancel(interface->pending_call);
- SUPPLICANT_DBG("Cancelled pending DBus call %p slot %d",
- interface->pending_call, interface->pending_slot);
-
- supplicant_dbus_call_callback(interface->pending_call,
- interface->pending_slot);
-
- interface->pending_call = NULL;
- interface->pending_slot = -1;
- }
+ SUPPLICANT_DBG("Cancelling any pending DBus calls");
+ supplicant_dbus_method_call_cancel_all(interface);
data = dbus_malloc0(sizeof(*data));
if (!data)
@@ -3010,7 +2985,7 @@ int g_supplicant_interface_remove(GSupplicantInterface
*interface,
"RemoveInterface",
interface_remove_params,
interface_remove_result, data,
- NULL, NULL);
+ NULL);
if (ret < 0) {
g_free(data->path);
dbus_free(data);
@@ -3033,8 +3008,6 @@ static void interface_scan_result(const char *error,
if (interface_exists(data->interface, data->path)) {
if (!data->interface->ready)
err = -ENOLINK;
-
- data->interface->pending_call = NULL;
}
g_free(data->path);
@@ -3231,7 +3204,7 @@ int g_supplicant_interface_scan(GSupplicantInterface
*interface,
ret = supplicant_dbus_method_call(interface->path,
SUPPLICANT_INTERFACE ".Interface", "Scan",
interface_scan_params, interface_scan_result, data,
- &interface->pending_call, &interface->pending_slot);
+ interface);
if (ret < 0) {
g_free(data->path);
@@ -3252,9 +3225,6 @@ static void interface_autoscan_result(const char *error,
err = -EIO;
}
- if (interface_exists(data->interface, data->interface->path))
- data->interface->pending_call = NULL;
-
g_free(data->path);
if (data->callback)
@@ -3293,8 +3263,7 @@ int g_supplicant_interface_autoscan(GSupplicantInterface
*interface,
SUPPLICANT_INTERFACE ".Interface", "AutoScan",
interface_autoscan_params,
interface_autoscan_result, data,
- &interface->pending_call,
- &interface->pending_slot);
+ interface);
if (ret < 0) {
g_free(data->path);
dbus_free(data);
@@ -3340,9 +3309,6 @@ static void interface_select_network_result(const char
*error,
err = parse_supplicant_error(iter);
}
- if (interface_exists(data->interface, data->path))
- data->interface->pending_call = NULL;
-
g_free(data->path);
if (data->callback)
@@ -3386,8 +3352,7 @@ static void interface_add_network_result(const char
*error,
SUPPLICANT_INTERFACE ".Interface", "SelectNetwork",
interface_select_network_params,
interface_select_network_result, data,
- &interface->pending_call,
- &interface->pending_slot);
+ interface);
return;
@@ -3395,8 +3360,6 @@ error:
SUPPLICANT_DBG("AddNetwork error %s", error);
if (interface_exists(data->interface, data->interface->path)) {
- interface->pending_call = NULL;
-
err = parse_supplicant_error(iter);
if (data->callback)
data->callback(err, data->interface, data->user_data);
@@ -3890,7 +3853,7 @@ static void wps_start(const char *error, DBusMessageIter
*iter, void *user_data)
supplicant_dbus_method_call(data->interface->path,
SUPPLICANT_INTERFACE ".Interface.WPS", "Start",
interface_add_wps_params,
- interface_wps_start_result, data, NULL, NULL);
+ interface_wps_start_result, data, NULL);
}
static void wps_process_credentials(DBusMessageIter *iter, void *user_data)
@@ -3943,8 +3906,7 @@ int g_supplicant_interface_connect(GSupplicantInterface
*interface,
SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
interface_add_network_params,
interface_add_network_result, data,
- &interface->pending_call,
- &interface->pending_slot);
+ interface);
if (ret < 0) {
g_free(data->path);
@@ -3970,9 +3932,6 @@ static void network_remove_result(const char *error,
result = -ECONNABORTED;
}
- if (interface_exists(data->interface, data->path))
- data->interface->pending_call = NULL;
-
g_free(data->path);
if (data->callback)
@@ -4000,7 +3959,7 @@ static int network_remove(struct interface_data *data)
return supplicant_dbus_method_call(interface->path,
SUPPLICANT_INTERFACE ".Interface", "RemoveNetwork",
network_remove_params, network_remove_result, data,
- &interface->pending_call, &interface->pending_slot);
+ interface);
}
static void interface_disconnect_result(const char *error,
@@ -4018,9 +3977,6 @@ static void interface_disconnect_result(const char *error,
result = -ECONNABORTED;
}
- if (interface_exists(data->interface, data->path))
- data->interface->pending_call = NULL;
-
if (result < 0 && data->callback) {
data->callback(result, data->interface, data->user_data);
data->callback = NULL;
@@ -4073,7 +4029,7 @@ int
g_supplicant_interface_disconnect(GSupplicantInterface *interface,
ret = supplicant_dbus_method_call(interface->path,
SUPPLICANT_INTERFACE ".Interface", "Disconnect",
NULL, interface_disconnect_result, data,
- &interface->pending_call, &interface->pending_slot);
+ interface);
if (ret < 0) {
g_free(data->path);
@@ -4099,7 +4055,6 @@ static void interface_p2p_find_result(const char *error,
err = -ENOLINK;
if (!err)
data->interface->p2p_finding = true;
- data->interface->pending_call = NULL;
}
if (data->callback)
@@ -4143,8 +4098,7 @@ int g_supplicant_interface_p2p_find(GSupplicantInterface
*interface,
ret = supplicant_dbus_method_call(interface->path,
SUPPLICANT_INTERFACE ".Interface.P2PDevice", "Find",
interface_p2p_find_params, interface_p2p_find_result,
- data, &interface->pending_call,
- &interface->pending_slot);
+ data, interface);
if (ret < 0) {
g_free(data->path);
dbus_free(data);
@@ -4164,7 +4118,7 @@ int
g_supplicant_interface_p2p_stop_find(GSupplicantInterface *interface)
return supplicant_dbus_method_call(interface->path,
SUPPLICANT_INTERFACE ".Interface.P2PDevice", "StopFind",
- NULL, NULL, NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
}
static const char *g_supplicant_rule0 = "type=signal,"
@@ -4267,7 +4221,7 @@ static void unregister_remove_interface(gpointer key,
gpointer value,
SUPPLICANT_INTERFACE,
"RemoveInterface",
unregister_interface_remove_params,
- NULL, interface->path, NULL, NULL);
+ NULL, interface->path, NULL);
}
void g_supplicant_unregister(const GSupplicantCallbacks *callbacks)
--
1.8.5.3
_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
