From 20085b9da8cada6e59b8d08c0a05f710add71708 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 11 Oct 2014 13:45:23 +0200 Subject: [PATCH 1/4] core: ignore SIGPIPE Ignoring SIGPIPE signal, otherwise it causes problems. For example, running `NetworkManager --debug 2>&1 | tee log.txt` in a terminal and killing it with CTRL+C (SIGINT), will abruplty terminate NetworkManager without clean shutdown. Note, that with this patch and above example, NetworkManager will both receive SIGINT and SIGPIPE. Since we now ignore SIGPIPE, NetworkManager will shut down cleanly. Any logging output after killing `tee` is of lost however. Also, there might be other cases where NM reads/writes to a pipe/socket and unexpectedly received SIGPIPE. For example nm-dns-manager.c spawns netconfig (run_netconfig()) and writes the configuration to its stdin. If netconfig dies, the write might fail with EPIPE. Signed-off-by: Thomas Haller --- src/main.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main.c b/src/main.c index 17769587a9..f4f693d1e4 100644 --- a/src/main.c +++ b/src/main.c @@ -99,6 +99,9 @@ signal_handling_thread (void *arg) /* Reread config stuff like system config files, VPN service files, etc */ nm_log_info (LOGD_CORE, "caught signal %d, not supported yet.", signo); break; + case SIGPIPE: + /* silently ignore signal */ + break; default: nm_log_err (LOGD_CORE, "caught unexpected signal %d", signo); break; @@ -124,6 +127,7 @@ setup_signals (void) sigaddset (&signal_set, SIGHUP); sigaddset (&signal_set, SIGINT); sigaddset (&signal_set, SIGTERM); + sigaddset (&signal_set, SIGPIPE); /* Block all signals of interest. */ status = pthread_sigmask (SIG_BLOCK, &signal_set, &old_sig_mask); From a7afa746f56ac9209dee589d51f8243378182f83 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 13 Oct 2014 20:07:43 +0200 Subject: [PATCH 2/4] supplicant: avoid assertion when DBUS connection closes Calling dbus_g_proxy_begin_call() on a closed DBUS connection will return NULL. All the call sites of nm_call_store_add() don't check for NULL and therefore might hit an assertion. This can easily reproduced by stopping the DBUS daemon. Backtrace: #0 0x000000381d0504e9 in g_logv (log_domain=0x59cd8b "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=, args=args@entry=0x7fff42cce5c0) at gmessages.c:989 #1 0x000000381d05063f in g_log (log_domain=, log_level=, format=) at gmessages.c:1025 #2 0x00000000004b64e9 in nm_call_store_add (store=0x7f8e1c003d20, proxy=0x219c0d0, call=0x0) at supplicant-manager/nm-call-store.c:47 #3 0x00000000004b0b7b in interface_add (self=0x20e2500, is_wireless=1) at supplicant-manager/nm-supplicant-interface.c:907 #4 0x00000000004b0865 in nm_supplicant_interface_new (smgr=0x216c870, ifname=0x211e840 "wlp3s0", is_wireless=1, fast_supported=1, ap_support=AP_SUPPORT_YES, start_now=1) at supplicant-manager/nm-supplicant-interface.c:1355 #5 0x00000000004b47da in nm_supplicant_manager_iface_get (self=0x216c870, ifname=0x211e840 "wlp3s0", is_wireless=1) at supplicant-manager/nm-supplicant-manager.c:91 #6 0x00007f8e250f8b3f in supplicant_interface_acquire (self=0x218a350) at nm-device-wifi.c:253 #7 0x00007f8e250fc22e in supplicant_iface_state_cb (iface=0x20e2290, new_state=13, old_state=9, disconnect_reason=0, user_data=0x218a350) at nm-device-wifi.c:2274 #8 0x000000381dc05d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76 #9 0x000000381dc056bc in ffi_call (cif=cif@entry=0x7fff42cced00, fn=0x7f8e250fbb20 , rvalue=0x7fff42ccec30, avalue=avalue@entry=0x7fff42ccebb0) at ../src/x86/ffi64.c:522 #10 0x000000381e010f35 in g_cclosure_marshal_generic_va (closure=0x20fd2b0, return_value=0x0, instance=0x20e2290, args_list=, marshal_data=0x0, n_params=3, param_types=0x2189ee0) at gclosure.c:1550 #11 0x000000381e0104c7 in _g_closure_invoke_va (closure=closure@entry=0x20fd2b0, return_value=return_value@entry=0x0, instance=instance@entry=0x20e2290, args=args@entry=0x7fff42ccef40, n_params=3, param_types=0x2189ee0) at gclosure.c:840 #12 0x000000381e029749 in g_signal_emit_valist (instance=0x20e2290, signal_id=, detail=0, var_args=var_args@entry=0x7fff42ccef40) at gsignal.c:3238 #13 0x000000381e02a3af in g_signal_emit (instance=, signal_id=, detail=) at gsignal.c:3386 #14 0x00000000004b0e4b in set_state (self=0x20e2290, new_state=13) at supplicant-manager/nm-supplicant-interface.c:344 Signed-off-by: Thomas Haller --- src/supplicant-manager/nm-call-store.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/supplicant-manager/nm-call-store.c b/src/supplicant-manager/nm-call-store.c index 4b427549d5..950d199e3a 100644 --- a/src/supplicant-manager/nm-call-store.c +++ b/src/supplicant-manager/nm-call-store.c @@ -44,7 +44,13 @@ nm_call_store_add (NMCallStore *store, g_return_if_fail (store != NULL); g_return_if_fail (proxy != NULL); - g_return_if_fail (call != NULL); + + if (!call) { + /* Allow calling nm_call_store_add() with NULL @call for convenience. + * This way you can pass the result of dbus_g_proxy_begin_call() directly + * to nm_call_store_add() without checking for NULL. */ + return; + } calls = g_hash_table_lookup (store, proxy); if (!calls) { From c7227ac7347383985312a8a827dfa28dbfc9767f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 13 Oct 2014 20:26:54 +0200 Subject: [PATCH 3/4] wifi: avoid assertion about duplicate nm_device_add_pending_action() This can be triggered by stopping the DBUS service. The assertion happens because when the supplicant stops (due to the name-owner-change, which is triggered because dbus-daemon quit), the supplicant manager sets all supplicant interfaces to DOWN state so that they can be cleaned up. That does two things: 1) calls supplicant_interface_acquire() to attempt to re-launch wpa_supplicant in case wpa_supplicant segfaulted 2) moves the NMDevicWifi to UNAVAILABLE state because the supplicant is gone, the device is no longer usable and we must terminate the connection and wait for the supplicant to come back But #2 also ends up calling supplicant_interface_acquire(), because that's what we want to do when the NMDeviceWifi is first managed (at startup) and when the supplicant dies. The code just doesn't differentiate between the two cases. To fix this, just allow duplicate "waiting for supplicant" pending actions, which is fine because the operation doesn't care about strict added/removed sequencing. #0 0x000000381d0504e9 in g_logv (log_domain=0x59cd5b "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=, args=args@entry=0x7fff8cccc1a0) at gmessages.c:989 #1 0x000000381d05063f in g_log (log_domain=, log_level=, format=) at gmessages.c:1025 #2 0x000000000044f53b in nm_device_add_pending_action (self=0xa60310, action=0x7febecd1d37d "waiting for supplicant", assert_not_yet_pending=1) at devices/nm-device.c:6466 #3 0x00007febecd0bc56 in supplicant_interface_acquire (self=0xa60310) at nm-device-wifi.c:262 #4 0x00007febecd0b240 in device_state_changed (device=0xa60310, new_state=NM_DEVICE_STATE_UNAVAILABLE, old_state=NM_DEVICE_STATE_ACTIVATED, reason=NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED) at nm-device-wifi.c:3136 #5 0x000000381dc05d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76 #6 0x000000381dc056bc in ffi_call (cif=cif@entry=0x7fff8cccc6c0, fn=0x7febecd0b050 , rvalue=0x7fff8cccc630, avalue=avalue@entry=0x7fff8cccc5b0) at ../src/x86/ffi64.c:522 #7 0x000000381e010ad8 in g_cclosure_marshal_generic (closure=0xa30a40, return_gvalue=0x0, n_param_values=, param_values=, invocation_hint=, marshal_data=0x7febecd0b050 ) at gclosure.c:1454 #8 0x000000381e010298 in g_closure_invoke (closure=closure@entry=0xa30a40, return_value=return_value@entry=0x0, n_param_values=4, param_values=param_values@entry=0x7fff8cccc8c0, invocation_hint=invocation_hint@entry=0x7fff8cccc860) at gclosure.c:777 #9 0x000000381e02211b in signal_emit_unlocked_R (node=node@entry=0xa322b0, detail=detail@entry=0, instance=instance@entry=0xa60310, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fff8cccc8c0) at gsignal.c:3624 #10 0x000000381e02a0f2 in g_signal_emit_valist (instance=instance@entry=0xa60310, signal_id=signal_id@entry=63, detail=detail@entry=0, var_args=var_args@entry=0x7fff8ccccaf8) at gsignal.c:3330 #11 0x000000381e02a8f8 in g_signal_emit_by_name (instance=0xa60310, detailed_signal=0x59a8d1 "state-changed") at gsignal.c:3426 #12 0x00000000004514a4 in _set_state_full (self=0xa60310, state=NM_DEVICE_STATE_UNAVAILABLE, reason=NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED, quitting=0) at devices/nm-device.c:6820 #13 0x0000000000449ec6 in nm_device_state_changed (self=0xa60310, state=NM_DEVICE_STATE_UNAVAILABLE, reason=NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED) at devices/nm-device.c:6949 #14 0x00007febecd0f247 in supplicant_iface_state_cb (iface=0x9b9290, new_state=13, old_state=12, disconnect_reason=0, user_data=0xa60310) at nm-device-wifi.c:2276 #15 0x000000381dc05d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76 #16 0x000000381dc056bc in ffi_call (cif=cif@entry=0x7fff8cccd230, fn=0x7febecd0eb20 , rvalue=0x7fff8cccd160, avalue=avalue@entry=0x7fff8cccd0e0) at ../src/x86/ffi64.c:522 #17 0x000000381e010f35 in g_cclosure_marshal_generic_va (closure=0xa2f490, return_value=0x0, instance=0x9b9290, args_list=, marshal_data=0x0, n_params=3, param_types=0xa422e0) at gclosure.c:1550 #18 0x000000381e0104c7 in _g_closure_invoke_va (closure=closure@entry=0xa2f490, return_value=return_value@entry=0x0, instance=instance@entry=0x9b9290, args=args@entry=0x7fff8cccd470, n_params=3, param_types=0xa422e0) at gclosure.c:840 #19 0x000000381e029749 in g_signal_emit_valist (instance=0x9b9290, signal_id=, detail=0, var_args=var_args@entry=0x7fff8cccd470) at gsignal.c:3238 #20 0x000000381e02a3af in g_signal_emit (instance=, signal_id=, detail=) at gsignal.c:3386 #21 0x00000000004b0e4b in set_state (self=0x9b9290, new_state=13) at supplicant-manager/nm-supplicant-interface.c:344 #22 0x00000000004b0916 in smgr_avail_cb (smgr=0xa3c890, pspec=0xa3c8d0, user_data=0x9b9290) at supplicant-manager/nm-supplicant-interface.c:930 #23 0x000000381e010298 in g_closure_invoke (closure=0x9a68b0, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7fff8cccd770, invocation_hint=invocation_hint@entry=0x7fff8cccd710) at gclosure.c:777 #24 0x000000381e02235d in signal_emit_unlocked_R (node=node@entry=0x990da0, detail=detail@entry=610, instance=instance@entry=0xa3c890, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fff8cccd770) at gsignal.c:3586 #25 0x000000381e02a0f2 in g_signal_emit_valist (instance=, signal_id=, detail=, var_args=var_args@entry=0x7fff8cccd900) at gsignal.c:3330 #26 0x000000381e02a3af in g_signal_emit (instance=, signal_id=, detail=) at gsignal.c:3386 #27 0x000000381e014945 in g_object_dispatch_properties_changed (object=0xa3c890, n_pspecs=1, pspecs=0x0) at gobject.c:1047 #28 0x000000381e017019 in g_object_notify_by_spec_internal (pspec=, object=0xa3c890) at gobject.c:1141 #29 g_object_notify (object=0xa3c890, property_name=) at gobject.c:1183 #30 0x00000000004b56f1 in set_running (self=0xa3c890, now_running=0) at supplicant-manager/nm-supplicant-manager.c:228 #31 0x00000000004b5002 in name_owner_changed (dbus_mgr=0x99f740, name=0x9ba910 "fi.w1.wpa_supplicant1", old_owner=0xa945a0 ":1.25", new_owner=0xac2ce0 "", user_data=0xa3c890) at supplicant-manager/nm-supplicant-manager.c:294 #32 0x000000381dc05d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76 #33 0x000000381dc056bc in ffi_call (cif=cif@entry=0x7fff8cccdd10, fn=0x4b4d50 , rvalue=0x7fff8cccdc80, avalue=avalue@entry=0x7fff8cccdc00) at ../src/x86/ffi64.c:522 #34 0x000000381e010ad8 in g_cclosure_marshal_generic (closure=0xa530a0, return_gvalue=0x0, n_param_values=, param_values=, invocation_hint=, marshal_data=0x0) at gclosure.c:1454 #35 0x000000381e010298 in g_closure_invoke (closure=0xa530a0, return_value=return_value@entry=0x0, n_param_values=4, param_values=param_values@entry=0x7fff8cccdf10, invocation_hint=invocation_hint@entry=0x7fff8cccdeb0) at gclosure.c:777 #36 0x000000381e02235d in signal_emit_unlocked_R (node=node@entry=0x99cce0, detail=detail@entry=0, instance=instance@entry=0x99f740, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fff8cccdf10) at gsignal.c:3586 #37 0x000000381e02a0f2 in g_signal_emit_valist (instance=, signal_id=, detail=, var_args=var_args@entry=0x7fff8ccce0d0) at gsignal.c:3330 #38 0x000000381e02a3af in g_signal_emit (instance=, signal_id=, detail=) at gsignal.c:3386 #39 0x00000000004c4026 in proxy_name_owner_changed (proxy=0x998210, name=0xa3ad50 "fi.w1.wpa_supplicant1", old_owner=0x9cffc0 ":1.25", new_owner=0x99d230 "", user_data=0x99f740) at nm-dbus-manager.c:708 #40 0x000000381dc05d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76 #41 0x000000381dc056bc in ffi_call (cif=cif@entry=0x7fff8ccce410, fn=0x4c3fd0 , rvalue=0x7fff8ccce380, avalue=avalue@entry=0x7fff8ccce300) at ../src/x86/ffi64.c:522 #42 0x000000381e010ad8 in g_cclosure_marshal_generic (closure=closure@entry=0x9beb80, return_gvalue=return_gvalue@entry=0x0, n_param_values=, param_values=, invocation_hint=invocation_hint@entry=0x7fff8ccce630, marshal_data=marshal_data@entry=0x0) at gclosure.c:1454 #43 0x0000003829a10864 in marshal_dbus_message_to_g_marshaller (closure=0x9beb80, return_value=0x0, n_param_values=, param_values=, invocation_hint=0x7fff8ccce630, marshal_data=0x0) at dbus-gproxy.c:1736 #44 0x000000381e010298 in g_closure_invoke (closure=0x9beb80, return_value=return_value@entry=0x0, n_param_values=3, param_values=param_values@entry=0x7fff8ccce690, invocation_hint=invocation_hint@entry=0x7fff8ccce630) at gclosure.c:777 #45 0x000000381e02235d in signal_emit_unlocked_R (node=node@entry=0x9be290, detail=detail@entry=347, instance=instance@entry=0x998210, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fff8ccce690) at gsignal.c:3586 #46 0x000000381e02a0f2 in g_signal_emit_valist (instance=, signal_id=, detail=, var_args=var_args@entry=0x7fff8ccce840) at gsignal.c:3330 #47 0x000000381e02a3af in g_signal_emit (instance=instance@entry=0x998210, signal_id=, detail=) at gsignal.c:3386 #48 0x0000003829a111c0 in dbus_g_proxy_emit_remote_signal (message=0xa6c2b0, proxy=0x998210) at dbus-gproxy.c:1789 #49 dbus_g_proxy_manager_filter (connection=, message=0xa6c2b0, user_data=0x9be520) at dbus-gproxy.c:1356 #50 0x000000382001006e in dbus_connection_dispatch (connection=connection@entry=0x9badb0) at dbus-connection.c:4631 #51 0x0000003829a0ad65 in message_queue_dispatch (source=source@entry=0x9bdcc0, callback=, user_data=) at dbus-gmain.c:90 #52 0x000000381d0492a6 in g_main_dispatch (context=0x99b4b0) at gmain.c:3066 #53 g_main_context_dispatch (context=context@entry=0x99b4b0) at gmain.c:3642 #54 0x000000381d049628 in g_main_context_iterate (context=0x99b4b0, block=block@entry=1, dispatch=dispatch@entry=1, self=) at gmain.c:3713 #55 0x000000381d049a3a in g_main_loop_run (loop=0x99b5d0) at gmain.c:3907 #56 0x0000000000443c28 in main (argc=1, argv=0x7fff8cccf268) at main.c:704 Signed-off-by: Thomas Haller --- src/devices/wifi/nm-device-wifi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 6617f4a69e..e3e927a5c7 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -246,7 +246,7 @@ supplicant_interface_acquire (NMDeviceWifi *self) } if (nm_supplicant_interface_get_state (priv->sup_iface) < NM_SUPPLICANT_INTERFACE_STATE_READY) - nm_device_add_pending_action (NM_DEVICE (self), "waiting for supplicant", TRUE); + nm_device_add_pending_action (NM_DEVICE (self), "waiting for supplicant", FALSE); g_signal_connect (priv->sup_iface, NM_SUPPLICANT_INTERFACE_STATE, From 3423e54c4e74b7a8c6b31450c5b5daadf6a2293f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 27 Oct 2014 16:14:56 +0100 Subject: [PATCH 4/4] settings: pass valid error code to g_set_error() in load_plugins() Fixes: 7d04618645f797d84c3f8692af6dfac1d67cb376 Signed-off-by: Thomas Haller --- src/settings/nm-settings.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 1bf2734457..e9d3eaf24d 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -650,7 +650,7 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) plugin = g_module_open (path, G_MODULE_BIND_LOCAL); if (!plugin) { - g_set_error (error, NM_SETTINGS_ERROR, 0, + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "Could not load plugin '%s': %s", pname, g_module_error ()); g_free (full_name); @@ -663,7 +663,7 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) g_free (path); if (!g_module_symbol (plugin, "nm_system_config_factory", (gpointer) (&factory_func))) { - g_set_error (error, NM_SETTINGS_ERROR, 0, + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "Could not find plugin '%s' factory function.", pname); success = FALSE; @@ -672,7 +672,7 @@ load_plugins (NMSettings *self, const char **plugins, GError **error) obj = (*factory_func) (); if (!obj || !NM_IS_SYSTEM_CONFIG_INTERFACE (obj)) { - g_set_error (error, NM_SETTINGS_ERROR, 0, + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "Plugin '%s' returned invalid system config object.", pname); success = FALSE;