From b75568ba24371051ae5bc30c87d47266637a6b4e Mon Sep 17 00:00:00 2001 From: Jiří Klimeš Date: Nov 20 2013 13:41:57 +0000 Subject: cherry-picked several fixes (for crashes) - dispatcher: fix crash on exit while logging from signal handler (rh #1017884) - core: workaround crash when connecting to wifi (rh #1025371) - ethernet: don't crash if device doesn't have a MAC address (rh #1029053) - libnm-glib: fix crash by taking additional ref in result_cb() (rh #1030403) - ifcfg-rh: fix ignoring updates that don't change anything --- diff --git a/NetworkManager.spec b/NetworkManager.spec index 9e8e1c4..0484c57 100644 --- a/NetworkManager.spec +++ b/NetworkManager.spec @@ -19,7 +19,7 @@ Name: NetworkManager Summary: Network connection manager and user applications Epoch: 1 Version: 0.9.9.0 -Release: 18%{snapshot}%{?dist} +Release: 19%{snapshot}%{?dist} Group: System Environment/Base License: GPLv2+ URL: http://www.gnome.org/projects/NetworkManager/ @@ -38,6 +38,11 @@ Patch8: rh1015598-wifi-detect.patch Patch9: nmcli-con-load.patch Patch10: rh1018317-vpn-logging.patch Patch11: rh1031170-bridge-port-crash.patch +Patch12: rh1017884-dispatcher-crash-on-exit.patch +Patch13: rh1025371-wifi-potential-crash.patch +Patch14: rh1029053-fix-crash-device-no-MAC.patch +Patch15: rh1030403-editor-crash-remote-connection.patch +Patch16: fix-ifcfg-rh-con-update.patch BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) @@ -172,6 +177,11 @@ deployments. %patch9 -p1 -b .nmcli-con-load %patch10 -p1 -b .vpn-log %patch11 -p1 -b .bridge-port +%patch12 -p1 -b .dispatcher +%patch13 -p1 -b .wifi-crash +%patch14 -p1 -b .device-no-MAC +%patch15 -p1 -b .libnm-glib-editor +%patch16 -p1 -b .ifcfg-rh-update %build @@ -370,6 +380,13 @@ fi %config %{_sysconfdir}/%{name}/conf.d/00-server.conf %changelog +* Wed Nov 20 2013 Jiří Klimeš - 0.9.9.0-19.git20131003 +- dispatcher: fix crash on exit while logging from signal handler (rh #1017884) +- core: workaround crash when connecting to wifi (rh #1025371) +- ethernet: don't crash if device doesn't have a MAC address (rh #1029053) +- libnm-glib: fix crash by taking additional ref in result_cb() (rh #1030403) +- ifcfg-rh: fix ignoring updates that don't change anything + * Mon Nov 18 2013 Dan Winship - 0.9.9.0-18.git20131003 - nmcli: add "con load" to manually load an ifcfg file - vpn: fix logging to help debug rh #1018317 diff --git a/fix-ifcfg-rh-con-update.patch b/fix-ifcfg-rh-con-update.patch new file mode 100644 index 0000000..1c13d78 --- /dev/null +++ b/fix-ifcfg-rh-con-update.patch @@ -0,0 +1,53 @@ +From 87041545b40fd0c0cfae16a8e605552b7715dc15 Mon Sep 17 00:00:00 2001 +From: Dan Williams +Date: Fri, 4 Oct 2013 23:38:31 -0500 +Subject: [PATCH] ifcfg-rh: fix ignoring updates that don't change anything +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +connection_from_file() requires the 'error' parameter. Not passing a +valid 'error' parameter causes the function to fail and return NULL, +which mean that commit_changes() would always re-write the connection +instead of ignoring commits where nothing has actually changed. + +connection_from_file() no longer requires the unmanaged, keyfile, +or routefile parameters, so remove them. + +Signed-off-by: Jiří Klimeš +--- + src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c | 11 +++-------- + 1 file changed, 3 insertions(+), 8 deletions(-) + +diff --git a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c +index 3788149..d3b93c9 100644 +--- a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c ++++ b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c +@@ -246,7 +246,6 @@ commit_changes (NMSettingsConnection *connection, + NMIfcfgConnectionPrivate *priv = NM_IFCFG_CONNECTION_GET_PRIVATE (connection); + GError *error = NULL; + NMConnection *reread; +- char *unmanaged = NULL, *keyfile = NULL, *routefile = NULL, *route6file = NULL; + gboolean same = FALSE, success = FALSE; + char *ifcfg_path = NULL; + +@@ -256,13 +255,9 @@ commit_changes (NMSettingsConnection *connection, + */ + if (priv->path) { + reread = connection_from_file (priv->path, NULL, NULL, NULL, +- &unmanaged, &keyfile, &routefile, &route6file, +- NULL, NULL); +- g_free (unmanaged); +- g_free (keyfile); +- g_free (routefile); +- g_free (route6file); +- ++ NULL, NULL, NULL, NULL, ++ &error, NULL); ++ g_clear_error (&error); + if (reread) { + same = nm_connection_compare (NM_CONNECTION (connection), + reread, +-- +1.7.11.7 + diff --git a/rh1017884-dispatcher-crash-on-exit.patch b/rh1017884-dispatcher-crash-on-exit.patch new file mode 100644 index 0000000..436df2f --- /dev/null +++ b/rh1017884-dispatcher-crash-on-exit.patch @@ -0,0 +1,88 @@ +From 073cc01f52f8b2b6d5b20c63814dc1ed00699028 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Mon, 18 Nov 2013 23:37:58 +0100 +Subject: [PATCH] dispatcher: fix crash while logging from signal handler +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Bug rh#1017884 describes a crash, where dbus_init() failed, which causes +a g_warning(). While writing the warning, a SIGTERM hit, and the +signal_handler() tries to call again g_message(). + +The logging functions of glib are not reentrant and call abort() when +invoked recursivly. The solution, is to use g_unix_signal_add, which +will dispatch the handler on the mainloop asynchronously. + +This bug is not that serious, because the dispatcher was about to +terminate anyway. However, it gets registered as a crash by the system +(ABRT). + +https://bugzilla.redhat.com/show_bug.cgi?id=1017884 + +Signed-off-by: Thomas Haller +Signed-off-by: Jiří Klimeš +--- + callouts/nm-dispatcher-action.c | 30 ++++++++++-------------------- + 1 file changed, 10 insertions(+), 20 deletions(-) + +diff --git a/callouts/nm-dispatcher-action.c b/callouts/nm-dispatcher-action.c +index 397f2ba..f48ff0a 100644 +--- a/callouts/nm-dispatcher-action.c ++++ b/callouts/nm-dispatcher-action.c +@@ -31,6 +31,7 @@ + #include + + #include ++#include + #include + #include + #include +@@ -597,27 +598,15 @@ logging_shutdown (void) + closelog (); + } + +-static void +-signal_handler (int signo) ++static gboolean ++signal_handler (gpointer user_data) + { +- if (signo == SIGINT || signo == SIGTERM) { +- g_message ("Caught signal %d, shutting down...", signo); +- g_main_loop_quit (loop); +- } +-} ++ int signo = GPOINTER_TO_INT (user_data); + +-static void +-setup_signals (void) +-{ +- struct sigaction action; +- sigset_t mask; +- +- sigemptyset (&mask); +- action.sa_handler = signal_handler; +- action.sa_mask = mask; +- action.sa_flags = 0; +- sigaction (SIGTERM, &action, NULL); +- sigaction (SIGINT, &action, NULL); ++ g_message ("Caught signal %d, shutting down...", signo); ++ g_main_loop_quit (loop); ++ ++ return G_SOURCE_REMOVE; + } + + int +@@ -648,7 +637,8 @@ main (int argc, char **argv) + g_option_context_free (opt_ctx); + + g_type_init (); +- setup_signals (); ++ g_unix_signal_add (SIGTERM, signal_handler, GINT_TO_POINTER (SIGTERM)); ++ g_unix_signal_add (SIGINT, signal_handler, GINT_TO_POINTER (SIGINT)); + + if (!debug) + logging_setup (); +-- +1.7.11.7 + diff --git a/rh1025371-wifi-potential-crash.patch b/rh1025371-wifi-potential-crash.patch new file mode 100644 index 0000000..d1597c7 --- /dev/null +++ b/rh1025371-wifi-potential-crash.patch @@ -0,0 +1,92 @@ +From 788eed99de51cd35adeb6585379b5e920c79b3f3 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Fri, 1 Nov 2013 10:32:27 +0100 +Subject: [PATCH] core: workaround crash when connecting to wifi (rh #1025371) +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +rh #1025371 reports a crash in handle_ip_config_timeout() because +nm_device_wifi_get_activation_ap() did not return any access point. + +The handling of the AP in nm-device-wifi.c should be reworked and soon +will be fixed. For now, play it safe, and try to cope with any cases +where nm_device_wifi_get_activation_ap() might return NULL. + +Later, this patch should be reverted and handling of the AP properly +cleaned up. + +https://bugzilla.redhat.com/show_bug.cgi?id=1025371 + +Signed-off-by: Thomas Haller +Signed-off-by: Jiří Klimeš +--- + src/devices/nm-device-wifi.c | 16 +++++++++------- + 1 file changed, 9 insertions(+), 7 deletions(-) + +diff --git a/src/devices/nm-device-wifi.c b/src/devices/nm-device-wifi.c +index 855c1e7..6c19d62 100644 +--- a/src/devices/nm-device-wifi.c ++++ b/src/devices/nm-device-wifi.c +@@ -2313,7 +2313,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, + */ + if (devstate == NM_DEVICE_STATE_CONFIG) { + NMAccessPoint *ap = nm_device_wifi_get_activation_ap (self); +- const GByteArray *ssid = nm_ap_get_ssid (ap); ++ const GByteArray *ssid = ap ? nm_ap_get_ssid (ap) : NULL; + + nm_log_info (LOGD_DEVICE | LOGD_WIFI, + "Activation (%s/wireless) Stage 2 of 5 (Device Configure) " +@@ -2593,9 +2593,8 @@ supplicant_connection_timeout_cb (gpointer user_data) + + g_assert (priv->mode == NM_802_11_MODE_INFRA); + ap = nm_device_wifi_get_activation_ap (self); +- g_assert (ap); + +- if (priv->ssid_found && is_encrypted (ap, connection)) { ++ if (priv->ssid_found && ap && is_encrypted (ap, connection)) { + guint64 timestamp = 0; + gboolean new_secrets = TRUE; + +@@ -2944,7 +2943,11 @@ act_stage2_config (NMDevice *dev, NMDeviceStateReason *reason) + g_assert (req); + + ap = nm_device_wifi_get_activation_ap (self); +- g_assert (ap); ++ if (!ap) { ++ nm_log_warn (LOGD_DEVICE | LOGD_WIFI, "act_stage2_config failed due to unexpected missing activation_ap. Abort"); ++ *reason = NM_DEVICE_STATE_REASON_SUPPLICANT_DISCONNECT; ++ goto out; ++ } + + connection = nm_act_request_get_connection (req); + g_assert (connection); +@@ -3102,7 +3105,6 @@ handle_ip_config_timeout (NMDeviceWifi *self, + } + + ap = nm_device_wifi_get_activation_ap (self); +- g_assert (ap); + + /* If IP configuration times out and it's a static WEP connection, that + * usually means the WEP key is wrong. WEP's Open System auth mode has +@@ -3111,7 +3113,7 @@ handle_ip_config_timeout (NMDeviceWifi *self, + * types (open, WPA, 802.1x, etc) if the secrets/certs were wrong the + * connection would have failed before IP configuration. + */ +- if (is_static_wep (ap, connection) && (may_fail == FALSE)) { ++ if (ap && is_static_wep (ap, connection) && (may_fail == FALSE)) { + /* Activation failed, we must have bad encryption key */ + nm_log_warn (LOGD_DEVICE | LOGD_WIFI, + "Activation (%s/wireless): could not get IP configuration for " +@@ -3203,7 +3205,7 @@ activation_success_handler (NMDevice *dev) + /* If the AP isn't fake, it was found in the scan list and all its + * details are known. + */ +- if (!nm_ap_get_fake (ap)) ++ if (!ap || !nm_ap_get_fake (ap)) + goto done; + + /* If the activate AP was fake, it probably won't have a BSSID at all. +-- +1.7.11.7 + diff --git a/rh1029053-fix-crash-device-no-MAC.patch b/rh1029053-fix-crash-device-no-MAC.patch new file mode 100644 index 0000000..fdac258 --- /dev/null +++ b/rh1029053-fix-crash-device-no-MAC.patch @@ -0,0 +1,73 @@ +From 696f655d7c7b605d0344aeb6ba4ff643cd46a5b4 Mon Sep 17 00:00:00 2001 +From: Dan Williams +Date: Mon, 11 Nov 2013 15:43:13 -0600 +Subject: [PATCH] ethernet: don't crash if device doesn't have a MAC address + (rh #1029053) +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Like IBM s390 CTC devices, which aren't really ethernet but for +historical reasons we treat them as such. + +Signed-off-by: Jiří Klimeš + +Updated to apply for Fedora 20 snapshot. + +--- + src/devices/nm-device-ethernet.c | 14 +++++++++++--- + 1 file changed, 11 insertions(+), 3 deletions(-) + +diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c +index 7b1c248..c0b7861 100644 +--- a/src/devices/nm-device-ethernet.c ++++ b/src/devices/nm-device-ethernet.c +@@ -310,6 +310,7 @@ update_permanent_hw_address (NMDevice *dev) + struct ifreq req; + struct ethtool_perm_addr *epaddr = NULL; + int fd, ret; ++ const guint8 *mac; + + fd = socket (PF_INET, SOCK_DGRAM, 0); + if (fd < 0) { +@@ -332,7 +333,11 @@ update_permanent_hw_address (NMDevice *dev) + nm_log_dbg (LOGD_HW | LOGD_ETHER, "(%s): unable to read permanent MAC address (error %d)", + nm_device_get_iface (dev), errno); + /* Fall back to current address */ +- memcpy (epaddr->data, nm_device_get_hw_address (dev, NULL), ETH_ALEN); ++ mac = nm_device_get_hw_address (dev, NULL); ++ if (mac) ++ memcpy (epaddr->data, mac, ETH_ALEN); ++ else ++ memset (epaddr->data, 0, ETH_ALEN); + } + + if (memcmp (&priv->perm_hw_addr, epaddr->data, ETH_ALEN)) { +@@ -350,11 +355,14 @@ update_initial_hw_address (NMDevice *dev) + NMDeviceEthernet *self = NM_DEVICE_ETHERNET (dev); + NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); + char *mac_str; ++ const guint8 *mac; + + /* This sets initial MAC address from current MAC address. It should only + * be called from NMDevice constructor() to really get the initial address. + */ +- memcpy (priv->initial_hw_addr, nm_device_get_hw_address (dev, NULL), ETH_ALEN); ++ mac = nm_device_get_hw_address (dev, NULL); ++ if (mac) ++ memcpy (priv->initial_hw_addr, mac, ETH_ALEN); + + mac_str = nm_utils_hwaddr_ntoa (priv->initial_hw_addr, ARPHRD_ETHER); + nm_log_dbg (LOGD_DEVICE | LOGD_ETHER, "(%s): read initial MAC address %s", +@@ -1219,7 +1219,7 @@ update_connection (NMDevice *device, NMConnection *connection) + { + NMSettingWired *s_wired = nm_connection_get_setting_wired (connection); + guint maclen; +- gconstpointer mac = nm_device_get_hw_address (device, &maclen); ++ const guint8 *mac = nm_device_get_hw_address (device, &maclen); + + if (!s_wired) { + s_wired = (NMSettingWired *) nm_setting_wired_new (); +-- +1.7.11.7 + diff --git a/rh1030403-editor-crash-remote-connection.patch b/rh1030403-editor-crash-remote-connection.patch new file mode 100644 index 0000000..5fcc7f7 --- /dev/null +++ b/rh1030403-editor-crash-remote-connection.patch @@ -0,0 +1,45 @@ +From fbcabeb7f72b710a790ca8617f7406a1ba7cf5be Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Mon, 18 Nov 2013 22:20:05 +0100 +Subject: [PATCH] libnm-glib: fix crash by taking additional ref in + nm-remote-connection/result_cb +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +result_cb invokes a function pointer provided by the user. Nothing prevents +the user from destroying the NMRemoteConnection in the callback, which leads +to a crash. Take an additional ref of NMRemoteConnection to keep it +alive. + +This probably caused a crash for nm-applet: +https://bugzilla.redhat.com/show_bug.cgi?id=1030403 + +Signed-off-by: Thomas Haller +Signed-off-by: Jiří Klimeš +--- + libnm-glib/nm-remote-connection.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/libnm-glib/nm-remote-connection.c b/libnm-glib/nm-remote-connection.c +index a03a44b..73a2cc8 100644 +--- a/libnm-glib/nm-remote-connection.c ++++ b/libnm-glib/nm-remote-connection.c +@@ -137,12 +137,14 @@ result_cb (DBusGProxy *proxy, DBusGProxyCall *proxy_call, gpointer user_data) + RemoteCall *call = user_data; + NMRemoteConnectionResultFunc func = (NMRemoteConnectionResultFunc) call->callback; + GError *error = NULL; ++ NMRemoteConnection *self = g_object_ref (call->self); + + dbus_g_proxy_end_call (proxy, proxy_call, &error, G_TYPE_INVALID); + if (func) + (*func) (call->self, error, call->user_data); + g_clear_error (&error); + remote_call_complete (call->self, call); ++ g_object_unref (self); + } + + /** +-- +1.7.11.7 +