diff options
| author | Boleslaw Tokarski <boleslaw.tokarski@jolla.com> | 2020-11-13 13:01:37 +0100 |
|---|---|---|
| committer | Daniel Wagner <wagi@monom.org> | 2020-12-11 12:29:41 +0100 |
| commit | 8506b8afc449683ac5534660c9097b3bfde9ba7a (patch) | |
| tree | be49bf4db28d523d4d54799df7d3e33104bb8df1 /plugins | |
| parent | 9aede91e811adc1d57518bd7ef108871acfdcab8 (diff) | |
| download | connman-8506b8afc449683ac5534660c9097b3bfde9ba7a.tar.gz | |
vpn: Secure a race condition with flag
ConnMan crashes happens in cancel_host_resolv in plugins/vpn.c. It
seems that the function gets called twice on the same data.
cancel_host_resolv is called from 2 places - from remove_result and from
connection_destroy. remove_result is called from resolv_result
indirectly - through g_timeout_add_seconds(0,...). In connection_destroy
it is called directly as part of the cleanup routine.
I managed to trigger a situation where remove_result was called and the
resolv structure was cleaned and freed, and after that (due to me
killing connman-vpnd in an unfortunate moment), connection_destroy
kicked in and attempted to access data in resolv (NULL) pointer.
To fix the issue, I add a user_data structure element representing the
event ID of the scheduled resolv structure removal function. The
cancel_host_resolv function would then check if the resolv removal
function was scheduled, and would remove it from the loop, so it does
not get executed.
Diffstat (limited to 'plugins')
| -rw-r--r-- | plugins/vpn.c | 10 |
1 files changed, 8 insertions, 2 deletions
diff --git a/plugins/vpn.c b/plugins/vpn.c index 5668c004..47458915 100644 --- a/plugins/vpn.c +++ b/plugins/vpn.c @@ -94,6 +94,7 @@ struct connection_data { GResolv *resolv; guint resolv_id; + guint remove_resolv_id; }; static int set_string(struct connman_provider *provider, @@ -171,10 +172,15 @@ static char *get_ident(const char *path) static void cancel_host_resolv(struct connection_data *data) { - if (data->resolv_id != 0) + + if (data->remove_resolv_id) + g_source_remove(data->remove_resolv_id); + + if (data->resolv && data->resolv_id) g_resolv_cancel_lookup(data->resolv, data->resolv_id); data->resolv_id = 0; + data->remove_resolv_id = 0; g_resolv_unref(data->resolv); data->resolv = NULL; @@ -206,7 +212,7 @@ static void resolv_result(GResolvResultStatus status, * We cannot unref the resolver here as resolv struct is manipulated * by gresolv.c after we return from this callback. */ - g_idle_add(remove_resolv, data); + data->remove_resolv_id = g_idle_add(remove_resolv, data); data->resolv_id = 0; } |