summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBeniamino Galvani <bgalvani@redhat.com>2018-11-23 09:56:01 +0100
committerBeniamino Galvani <bgalvani@redhat.com>2018-11-29 17:53:11 +0100
commitfffd4d23db2c7c708d52b1a891870ce0d4dcd1de (patch)
tree6a47f11e583deb8f8a72cd468e326946537a2a3f
parent803514df27f647df49023f86be2326676715e2f8 (diff)
downloadNetworkManager-bg/cli-disconnect-rh1546061.tar.gz
cli: avoid crash on device disconnectbg/cli-disconnect-rh1546061
When nm_device_disconnect_async() returns, the device could be still in DEACTIVATING state, and so we also register to device-state signal notifications to know when the device state goes to DISCONNECTED. Sometimes it happens that the device state goes to DISCONNECTED before nm_device_disconnect_async() returns. In this case the signal handler exits the main loop and then the callback for disconnect_async() is executed anyway because it was already dispatched, leading to an invalid memory access. To avoid this we should cancel nm_device_disconnect_async() when we are quitting the main loop. Reproducer: nmcli connection add type team ifname t1 con-name t1 nmcli connection up t1 nmcli device disconnect t1 & nmcli device delete t1 Crash example: ==14955==ERROR: AddressSanitizer: SEGV on unknown address 0xffffffff0000000b (pc 0x7f128c8ba3dd bp 0x0000004be080 sp 0x7ffcda7dc6e0 T0) ==14955==The signal is caused by a READ memory access. 0 0x7f128c8ba3dc in g_string_truncate (/lib64/libglib-2.0.so.0+0x713dc) 1 0x7f128c8bb4bb in g_string_printf (/lib64/libglib-2.0.so.0+0x724bb) 2 0x45bdfa in disconnect_device_cb clients/cli/devices.c:2321 3 0x7f128ca3d1a9 in g_simple_async_result_complete /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gsimpleasyncresult.c:802 4 0x7f128cf85d0e in device_disconnect_cb libnm/nm-device.c:2354 5 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148 6 0x7f128ca508d5 in g_task_return /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1206 7 0x7f128ca8ecfc in reply_cb /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gdbusproxy.c:2586 8 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148 9 0x7f128ca508d5 in g_task_return /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1206 10 0x7f128ca83440 in g_dbus_connection_call_done /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gdbusconnection.c:5713 11 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148 12 0x7f128ca4ffac in complete_in_idle_cb /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1162 13 0x7f128c893b7a in g_idle_dispatch gmain.c:5620 14 0x7f128c89726c in g_main_dispatch gmain.c:3182 15 0x7f128c897637 in g_main_context_iterate gmain.c:3920 16 0x7f128c897961 in g_main_loop_run (/lib64/libglib-2.0.so.0+0x4e961) 17 0x473afb in main clients/cli/nmcli.c:1067 18 0x7f128c6a1412 in __libc_start_main (/lib64/libc.so.6+0x24412) 19 0x416c39 in _start (/usr/bin/nmcli+0x416c39) https://bugzilla.redhat.com/show_bug.cgi?id=1546061
-rw-r--r--clients/cli/devices.c13
1 files changed, 11 insertions, 2 deletions
diff --git a/clients/cli/devices.c b/clients/cli/devices.c
index d9b84c1b07..699f83048e 100644
--- a/clients/cli/devices.c
+++ b/clients/cli/devices.c
@@ -2064,6 +2064,7 @@ typedef struct {
GSList *queue;
guint timeout_id;
gboolean cmd_disconnect;
+ GCancellable *cancellable;
} DeviceCbInfo;
static void device_cb_info_finish (DeviceCbInfo *info, NMDevice *device);
@@ -2136,7 +2137,10 @@ device_cb_info_finish (DeviceCbInfo *info, NMDevice *device)
if (info->timeout_id)
g_source_remove (info->timeout_id);
+
g_signal_handlers_disconnect_by_func (info->nmc->client, device_removed_cb, info);
+ nm_clear_g_cancellable (&info->cancellable);
+
g_slice_free (DeviceCbInfo, info);
quit ();
}
@@ -2313,11 +2317,14 @@ disconnect_device_cb (GObject *object, GAsyncResult *result, gpointer user_data)
{
NMDevice *device = NM_DEVICE (object);
DeviceCbInfo *info = (DeviceCbInfo *) user_data;
- NmCli *nmc = info->nmc;
+ NmCli *nmc;
NMDeviceState state;
GError *error = NULL;
if (!nm_device_disconnect_finish (device, result, &error)) {
+ if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ return;
+ nmc = info->nmc;
g_string_printf (nmc->return_text, _("Error: not all devices disconnected."));
g_printerr (_("Error: Device '%s' (%s) disconnecting failed: %s\n"),
nm_device_get_iface (device),
@@ -2327,6 +2334,7 @@ disconnect_device_cb (GObject *object, GAsyncResult *result, gpointer user_data)
nmc->return_value = NMC_RESULT_ERROR_DEV_DISCONNECT;
device_cb_info_finish (info, device);
} else {
+ nmc = info->nmc;
state = nm_device_get_state (device);
if (nmc->nowait_flag || state <= NM_DEVICE_STATE_DISCONNECTED) {
/* Don't want to wait or device already disconnected */
@@ -2363,6 +2371,7 @@ do_devices_disconnect (NmCli *nmc, int argc, char **argv)
info = g_slice_new0 (DeviceCbInfo);
info->nmc = nmc;
info->cmd_disconnect = TRUE;
+ info->cancellable = g_cancellable_new ();
if (nmc->timeout > 0)
info->timeout_id = g_timeout_add_seconds (nmc->timeout, device_op_timeout_cb, info);
@@ -2380,7 +2389,7 @@ do_devices_disconnect (NmCli *nmc, int argc, char **argv)
G_CALLBACK (disconnect_state_cb), info);
/* Now disconnect the device */
- nm_device_disconnect_async (device, NULL, disconnect_device_cb, info);
+ nm_device_disconnect_async (device, info->cancellable, disconnect_device_cb, info);
}
out: