Commit fa40fc6d authored by Thomas Haller's avatar Thomas Haller

connectivity: fix crash when removing easy-handle from curl callback

libcurl does not allow removing easy-handles from within a curl
callback.

That was already partly avoided for one handle alone. That is, when
a handle completed inside a libcurl callback, it would only invoke the
callback, but not yet delete it. However, that is not enough, because
from within a callback another handle can be cancelled, leading to
the removal of (the other) handle and a crash:

  ==24572==    at 0x40319AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==24572==    by 0x52DDAE5: Curl_close (url.c:392)
  ==24572==    by 0x52EC02C: curl_easy_cleanup (easy.c:825)
  ==24572==    by 0x5FDCD2: cb_data_free (nm-connectivity.c:215)
  ==24572==    by 0x5FF6DE: nm_connectivity_check_cancel (nm-connectivity.c:585)
  ==24572==    by 0x55F7F9: concheck_handle_complete (nm-device.c:2601)
  ==24572==    by 0x574C12: concheck_cb (nm-device.c:2725)
  ==24572==    by 0x5FD887: cb_data_invoke_callback (nm-connectivity.c:167)
  ==24572==    by 0x5FD959: easy_header_cb (nm-connectivity.c:435)
  ==24572==    by 0x52D73CB: chop_write (sendf.c:612)
  ==24572==    by 0x52D73CB: Curl_client_write (sendf.c:668)
  ==24572==    by 0x52D54ED: Curl_http_readwrite_headers (http.c:3904)
  ==24572==    by 0x52E9EA7: readwrite_data (transfer.c:548)
  ==24572==    by 0x52E9EA7: Curl_readwrite (transfer.c:1161)
  ==24572==    by 0x52F4193: multi_runsingle (multi.c:1915)
  ==24572==    by 0x52F5531: multi_socket (multi.c:2607)
  ==24572==    by 0x52F5804: curl_multi_socket_action (multi.c:2771)

Fix that, by never invoking any callbacks when we are inside a libcurl
callback. Instead, the handle is marked for completion and queued. Later,
we complete all queue handles separately.

While at it, drop the @error argument from NMConnectivityCheckCallback.
It was only used to signal cancellation. Let's instead signal that via
status NM_CONNECTIVITY_CANCELLED.

https://bugzilla.gnome.org/show_bug.cgi?id=797136
https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1792745
https://bugzilla.opensuse.org/show_bug.cgi?id=1107197
https://github.com/NetworkManager/NetworkManager/pull/207

Fixes: d8a31794
parent 0b3197a3
......@@ -2813,7 +2813,6 @@ static void
concheck_cb (NMConnectivity *connectivity,
NMConnectivityCheckHandle *c_handle,
NMConnectivityState state,
GError *error,
gpointer user_data)
{
_nm_unused gs_unref_object NMDevice *self_keep_alive = NULL;
......@@ -2834,7 +2833,7 @@ concheck_cb (NMConnectivity *connectivity,
handle->c_handle = NULL;
self = handle->self;
if (nm_utils_error_is_cancelled (error, FALSE)) {
if (state == NM_CONNECTIVITY_CANCELLED) {
/* the only place where we nm_connectivity_check_cancel(@c_handle), is
* from inside concheck_handle_complete(). This is a recursive call,
* nothing to do. */
......@@ -2843,15 +2842,14 @@ concheck_cb (NMConnectivity *connectivity,
return;
}
/* we keep NMConnectivity instance alive. It cannot be disposing. */
nm_assert (state != NM_CONNECTIVITY_DISPOSING);
self_keep_alive = g_object_ref (self);
_LOGT (LOGD_CONCHECK, "connectivity: complete check (seq:%llu, state:%s%s%s%s)",
_LOGT (LOGD_CONCHECK, "connectivity: complete check (seq:%llu, state:%s)",
(long long unsigned) handle->seq,
nm_connectivity_state_to_string (state),
NM_PRINT_FMT_QUOTED (error, ", error: ", error->message, "", ""));
/* we keep NMConnectivity instance alive. It cannot be disposing. */
nm_assert (!nm_utils_error_is_cancelled (error, TRUE));
nm_connectivity_state_to_string (state));
/* keep @self alive, while we invoke callbacks. */
priv = NM_DEVICE_GET_PRIVATE (self);
......
This diff is collapsed.
......@@ -24,8 +24,10 @@
#include "nm-dbus-interface.h"
#define NM_CONNECTIVITY_ERROR ((NMConnectivityState) -1)
#define NM_CONNECTIVITY_FAKE ((NMConnectivityState) -2)
#define NM_CONNECTIVITY_ERROR ((NMConnectivityState) -1)
#define NM_CONNECTIVITY_FAKE ((NMConnectivityState) -2)
#define NM_CONNECTIVITY_CANCELLED ((NMConnectivityState) -3)
#define NM_CONNECTIVITY_DISPOSING ((NMConnectivityState) -4)
#define NM_TYPE_CONNECTIVITY (nm_connectivity_get_type ())
#define NM_CONNECTIVITY(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_CONNECTIVITY, NMConnectivity))
......@@ -53,7 +55,6 @@ typedef struct _NMConnectivityCheckHandle NMConnectivityCheckHandle;
typedef void (*NMConnectivityCheckCallback) (NMConnectivity *self,
NMConnectivityCheckHandle *handle,
NMConnectivityState state,
GError *error,
gpointer user_data);
NMConnectivityCheckHandle *nm_connectivity_check_start (NMConnectivity *self,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment