From 170f1c14b71bcf700325a0cd55f46ebe669effde Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Dec 18 2013 19:03:30 +0000 Subject: Fix a couple bugs (rh #1034921) (rh #1029213) (rh #1032819) - core: wait for link before declaring startup complete (rh #1034921) - core: ignore RA-provided IPv6 default routes (rh #1029213) - core: set IPv4 broadcast address correctly (rh #1032819) --- diff --git a/NetworkManager.spec b/NetworkManager.spec index 360cf15..001c6e9 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: 20%{snapshot}%{?dist} +Release: 21%{snapshot}%{?dist} Group: System Environment/Base License: GPLv2+ URL: http://www.gnome.org/projects/NetworkManager/ @@ -45,6 +45,9 @@ Patch15: rh1030403-editor-crash-remote-connection.patch Patch16: fix-ifcfg-rh-con-update.patch Patch17: rh1018317-prereq.patch Patch18: rh1018317-openvpn-ptp.patch +Patch19: rh1034921-startup-link-wait.patch +Patch20: rh1029213-ignore-RA-default-routes.patch +Patch21: rh1032819-set-broadcast-address.patch BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) @@ -186,6 +189,9 @@ deployments. %patch16 -p1 -b .ifcfg-rh-update %patch17 -p1 -b .ipv6-flags %patch18 -p1 -b .openvpn-ptp +%patch19 -p1 -b .startup-linkwait +%patch20 -p1 -b .ignore-RA-default-routes +%patch21 -p1 -b .broadcast-addr %build @@ -384,6 +390,11 @@ fi %config %{_sysconfdir}/%{name}/conf.d/00-server.conf %changelog +* Thu Dec 12 2013 Dan Williams - 0.9.9.0-21.git20131003 +- core: wait for link before declaring startup complete (rh #1034921) +- core: ignore RA-provided IPv6 default routes (rh #1029213) +- core: set IPv4 broadcast address correctly (rh #1032819) + * Mon Dec 2 2013 Dan Winship - 0.9.9.0-20.git20131003 - core: Fix PtP/peer address support, for OpenVPN (rh #1018317) diff --git a/rh1029213-ignore-RA-default-routes.patch b/rh1029213-ignore-RA-default-routes.patch new file mode 100644 index 0000000..e5b06d4 --- /dev/null +++ b/rh1029213-ignore-RA-default-routes.patch @@ -0,0 +1,98 @@ +From 8586353b09460ec0a619058421743dd7d424a75d Mon Sep 17 00:00:00 2001 +From: Dan Williams +Date: Wed, 20 Nov 2013 13:40:07 -0600 +Subject: [PATCH] core: ignore RA-provided default routes (rh #1029213) + +The router has no idea what the local configuration or user preferences are, +so sending routes with a prefix length of 0 is at best misinformed and at +worst breaks things. The kernel also ignores plen=0 routes in its in-kernel +RA processing code in net/ipv6/ndisc.c. + +https://bugzilla.redhat.com/show_bug.cgi?id=1029213 +--- + src/devices/nm-device.c | 16 +++++++++++----- + 1 file changed, 11 insertions(+), 5 deletions(-) + +diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c +index f03ecbb..d92a94b 100644 +--- a/src/devices/nm-device.c ++++ b/src/devices/nm-device.c +@@ -3283,20 +3283,26 @@ rdisc_config_changed (NMRDisc *rdisc, NMRDiscConfigMap changed, NMDevice *device + /* Rebuild route list from router discovery cache. */ + nm_ip6_config_reset_routes (priv->ac_ip6_config); + + for (i = 0; i < rdisc->routes->len; i++) { + NMRDiscRoute *discovered_route = &g_array_index (rdisc->routes, NMRDiscRoute, i); + NMPlatformIP6Route route; + +- memset (&route, 0, sizeof (route)); +- route.network = discovered_route->network; +- route.plen = discovered_route->plen; +- route.gateway = discovered_route->gateway; ++ /* Only accept non-default routes. The router has no idea what the ++ * local configuration or user preferences are, so sending routes ++ * with a prefix length of 0 is quite rude and thus ignored. ++ */ ++ if (discovered_route->plen > 0) { ++ memset (&route, 0, sizeof (route)); ++ route.network = discovered_route->network; ++ route.plen = discovered_route->plen; ++ route.gateway = discovered_route->gateway; + +- nm_ip6_config_add_route (priv->ac_ip6_config, &route); ++ nm_ip6_config_add_route (priv->ac_ip6_config, &route); ++ } + } + } + + if (changed & NM_RDISC_CONFIG_DNS_SERVERS) { + /* Rebuild DNS server list from router discovery cache. */ + nm_ip6_config_reset_nameservers (priv->ac_ip6_config); + +-- +1.8.3.1 + +From 6e73f01b6e69f44f8d9da4872fb796b9d80acac1 Mon Sep 17 00:00:00 2001 +From: Dan Williams +Date: Tue, 3 Dec 2013 14:12:55 -0600 +Subject: [PATCH] platform: fix possible out-of-bounds access with RA route + masking + +If the prefix length was 128, that could cause an access beyond the +end of the array. Found by Thomas Haller. +--- + src/rdisc/nm-lndp-rdisc.c | 10 +++++++--- + 1 file changed, 7 insertions(+), 3 deletions(-) + +diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c +index abcc3c2..3299b32 100644 +--- a/src/rdisc/nm-lndp-rdisc.c ++++ b/src/rdisc/nm-lndp-rdisc.c +@@ -411,17 +411,21 @@ set_address_masked (struct in6_addr *dst, struct in6_addr *src, guint8 plen) + guint nbytes = plen / 8; + guint nbits = plen % 8; + + g_return_if_fail (plen <= 128); + g_assert (src); + g_assert (dst); + +- memset (dst, 0, sizeof (*dst)); +- memcpy (dst, src, nbytes); +- dst->s6_addr[nbytes] = (src->s6_addr[nbytes] & (0xFF << (8 - nbits))); ++ if (plen >= 128) ++ *dst = *src; ++ else { ++ memset (dst, 0, sizeof (*dst)); ++ memcpy (dst, src, nbytes); ++ dst->s6_addr[nbytes] = (src->s6_addr[nbytes] & (0xFF << (8 - nbits))); ++ } + } + + static int + receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) + { + NMRDisc *rdisc = (NMRDisc *) user_data; + NMLNDPRDiscPrivate *priv = NM_LNDP_RDISC_GET_PRIVATE (rdisc); +-- +1.8.3.1 + diff --git a/rh1032819-set-broadcast-address.patch b/rh1032819-set-broadcast-address.patch new file mode 100644 index 0000000..dd3c0fb --- /dev/null +++ b/rh1032819-set-broadcast-address.patch @@ -0,0 +1,86 @@ +From 7eb12a5b21f87d7592ec2c5235d1ed90c4fac132 Mon Sep 17 00:00:00 2001 +From: Dan Williams +Date: Tue, 3 Dec 2013 11:42:28 -0600 +Subject: [PATCH] platform: set IPv4 broadcast address too (rh #1032819) + +When moving over the platform, setting of the IPv4 broadcast address +got lost. Bring it back. + +https://bugzilla.redhat.com/show_bug.cgi?id=1032819 +--- + src/platform/nm-linux-platform.c | 32 ++++++++++++++++++++++++++++++++ + 1 file changed, 32 insertions(+) + +diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c +index 8f0e077..3f57925 100644 +--- a/src/platform/nm-linux-platform.c ++++ b/src/platform/nm-linux-platform.c +@@ -2213,14 +2213,33 @@ ip6_address_get_all (NMPlatform *platform, int ifindex) + nl_object_unmark (object); + } + } + + return addresses; + } + ++static void ++addr4_to_broadcast (struct in_addr *dst, const struct in_addr *src, guint8 plen) ++{ ++ guint nbytes = plen / 8; ++ guint nbits = plen % 8; ++ ++ g_return_if_fail (plen <= 32); ++ g_assert (src); ++ g_assert (dst); ++ ++ if (plen >= 32) ++ *dst = *src; ++ else { ++ dst->s_addr = 0xFFFFFFFF; ++ memcpy (dst, src, nbytes); ++ ((guint8 *) dst)[nbytes] = (((const guint8 *) src)[nbytes] | (0xFF >> nbits)); ++ } ++} ++ + static struct nl_object * + build_rtnl_addr (int family, + int ifindex, + gconstpointer addr, + gconstpointer peer_addr, + int plen, + guint32 lifetime, +@@ -2230,18 +2249,31 @@ build_rtnl_addr (int family, + struct rtnl_addr *rtnladdr = rtnl_addr_alloc (); + int addrlen = family == AF_INET ? sizeof (in_addr_t) : sizeof (struct in6_addr); + auto_nl_addr struct nl_addr *nladdr = nl_addr_build (family, addr, addrlen); + int nle; + + g_assert (rtnladdr && nladdr); + ++ /* IP address */ + rtnl_addr_set_ifindex (rtnladdr, ifindex); + nle = rtnl_addr_set_local (rtnladdr, nladdr); + g_assert (!nle); + ++ /* IPv4 Broadcast address */ ++ if (family == AF_INET) { ++ struct in_addr bcast; ++ auto_nl_addr struct nl_addr *bcaddr = NULL; ++ ++ addr4_to_broadcast (&bcast, addr, plen); ++ bcaddr = nl_addr_build (family, &bcast, addrlen); ++ g_assert (bcaddr); ++ rtnl_addr_set_broadcast (rtnladdr, bcaddr); ++ } ++ ++ /* Peer/point-to-point address */ + if (peer_addr) { + auto_nl_addr struct nl_addr *nlpeer = nl_addr_build (family, peer_addr, addrlen); + + nle = rtnl_addr_set_peer (rtnladdr, nlpeer); + g_assert (!nle); + } + +-- +1.8.3.1 + diff --git a/rh1034921-startup-link-wait.patch b/rh1034921-startup-link-wait.patch new file mode 100644 index 0000000..d851b1f --- /dev/null +++ b/rh1034921-startup-link-wait.patch @@ -0,0 +1,833 @@ +From 07281f21a71fa333288821ad863accfeedfff567 Mon Sep 17 00:00:00 2001 +From: Dan Williams +Date: Mon, 9 Dec 2013 12:55:04 -0600 +Subject: [PATCH] core: delay startup complete until carrier is found or + timeout (rh #1034921) (rh #1030583) + +--- + src/devices/nm-device.c | 177 ++++++++++++++++++++++++++++++++++----------- + src/nm-active-connection.c | 8 ++ + 2 files changed, 142 insertions(+), 43 deletions(-) + +diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c +index 578ccb0..96a5d44 100644 +--- a/src/devices/nm-device.c ++++ b/src/devices/nm-device.c +@@ -184,15 +184,15 @@ typedef struct { + gboolean initialized; + gboolean in_state_changed; + + NMDeviceState state; + NMDeviceStateReason state_reason; + QueuedState queued_state; + guint queued_ip_config_id; +- guint pending_actions; ++ GArray *pending_actions; + + char * udi; + char * path; + char * iface; /* may change, could be renamed by user */ + int ifindex; + gboolean is_software; + char * ip_iface; +@@ -224,14 +224,15 @@ typedef struct { + gpointer act_source6_func; + + /* Link stuff */ + guint link_connected_id; + guint link_disconnected_id; + guint carrier_defer_id; + gboolean carrier; ++ guint carrier_wait_id; + gboolean ignore_carrier; + + /* Generic DHCP stuff */ + NMDHCPManager * dhcp_manager; + guint32 dhcp_timeout; + GByteArray * dhcp_anycast_address; + +@@ -390,14 +391,15 @@ nm_device_init (NMDevice *self) + priv->capabilities = NM_DEVICE_CAP_NM_SUPPORTED; + priv->state = NM_DEVICE_STATE_UNMANAGED; + priv->state_reason = NM_DEVICE_STATE_REASON_NONE; + priv->dhcp_timeout = 0; + priv->rfkill_type = RFKILL_TYPE_UNKNOWN; + priv->autoconnect = DEFAULT_AUTOCONNECT; + priv->available_connections = g_hash_table_new_full (g_direct_hash, g_direct_equal, g_object_unref, NULL); ++ priv->pending_actions = g_array_sized_new (FALSE, TRUE, sizeof (GQuark), 5); + } + + static void + update_accept_ra_save (NMDevice *self) + { + NMDevicePrivate *priv; + const char *ip_iface; +@@ -1154,14 +1156,20 @@ nm_device_set_carrier (NMDevice *device, gboolean carrier) + if (priv->carrier) { + nm_log_info (LOGD_DEVICE, "(%s): link connected", iface); + if (priv->carrier_defer_id) { + g_source_remove (priv->carrier_defer_id); + priv->carrier_defer_id = 0; + } + klass->carrier_changed (device, TRUE); ++ ++ if (priv->carrier_wait_id) { ++ g_source_remove (priv->carrier_wait_id); ++ priv->carrier_wait_id = 0; ++ nm_device_remove_pending_action (device, "carrier wait"); ++ } + } else if (state <= NM_DEVICE_STATE_DISCONNECTED) { + nm_log_info (LOGD_DEVICE, "(%s): link disconnected", iface); + klass->carrier_changed (device, FALSE); + } else { + nm_log_info (LOGD_DEVICE, "(%s): link disconnected (deferring action for %d seconds)", + iface, LINK_DISCONNECT_DELAY); + priv->carrier_defer_id = g_timeout_add_seconds (LINK_DISCONNECT_DELAY, +@@ -5110,17 +5118,28 @@ nm_device_start_ip_check (NMDevice *self) + /* If no ping was started, just advance to SECONDARIES */ + if (!priv->gw_ping.pid) + nm_device_queue_state (self, NM_DEVICE_STATE_SECONDARIES, NM_DEVICE_STATE_REASON_NONE); + } + + /****************************************************************/ + ++static gboolean ++carrier_wait_timeout (gpointer user_data) ++{ ++ NMDevice *self = NM_DEVICE (user_data); ++ ++ NM_DEVICE_GET_PRIVATE (self)->carrier_wait_id = 0; ++ nm_device_remove_pending_action (self, "carrier wait"); ++ return G_SOURCE_REMOVE; ++} ++ + gboolean + nm_device_bring_up (NMDevice *self, gboolean block, gboolean *no_firmware) + { ++ NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + gboolean success; + guint32 tries = 0; + + g_return_val_if_fail (NM_IS_DEVICE (self), FALSE); + + if (nm_device_is_up (self)) + goto out; +@@ -5138,14 +5157,28 @@ nm_device_bring_up (NMDevice *self, gboolean block, gboolean *no_firmware) + g_usleep (200); + + if (!nm_device_is_up (self)) { + nm_log_warn (LOGD_HW, "(%s): device not up after timeout!", nm_device_get_iface (self)); + return FALSE; + } + ++ /* Devices that support carrier detect must be IFF_UP to report carrier ++ * changes; so after setting the device IFF_UP we must suppress startup ++ * complete (via a pending action) until either the carrier turns on, or ++ * a timeout is reached. ++ */ ++ if (device_has_capability (self, NM_DEVICE_CAP_CARRIER_DETECT)) { ++ if (priv->carrier_wait_id) { ++ g_source_remove (priv->carrier_wait_id); ++ nm_device_remove_pending_action (self, "carrier wait"); ++ } ++ priv->carrier_wait_id = g_timeout_add_seconds (5, carrier_wait_timeout, self); ++ nm_device_add_pending_action (self, "carrier wait"); ++ } ++ + out: + /* Can only get HW address of some devices when they are up */ + nm_device_update_hw_address (self); + + _update_ip4_address (self); + return TRUE; + } +@@ -5301,14 +5334,19 @@ dispose (GObject *object) + } + + if (priv->cp_updated_id) { + g_signal_handler_disconnect (priv->con_provider, priv->cp_updated_id); + priv->cp_updated_id = 0; + } + ++ if (priv->carrier_wait_id) { ++ g_source_remove (priv->carrier_wait_id); ++ priv->carrier_wait_id = 0; ++ } ++ + g_hash_table_unref (priv->available_connections); + priv->available_connections = NULL; + + activation_source_clear (self, TRUE, AF_INET); + activation_source_clear (self, TRUE, AF_INET6); + + clear_act_request (self); +@@ -5331,14 +5369,17 @@ finalize (GObject *object) + + if (priv->dhcp_manager) + g_object_unref (priv->dhcp_manager); + + if (priv->fw_manager) + g_object_unref (priv->fw_manager); + ++ g_array_free (priv->pending_actions, TRUE); ++ priv->pending_actions = NULL; ++ + g_free (priv->udi); + g_free (priv->path); + g_free (priv->iface); + g_free (priv->ip_iface); + g_free (priv->driver); + g_free (priv->driver_version); + g_free (priv->firmware_version); +@@ -5931,46 +5972,54 @@ nm_device_set_firmware_missing (NMDevice *self, gboolean new_missing) + + gboolean + nm_device_get_firmware_missing (NMDevice *self) + { + return NM_DEVICE_GET_PRIVATE (self)->firmware_missing; + } + ++#define QUEUED_PREFIX "queued state change to " ++ + static const char * +-state_to_string (NMDeviceState state) ++queued_state_to_string (NMDeviceState state) + { + switch (state) { + case NM_DEVICE_STATE_UNMANAGED: +- return "unmanaged"; ++ return QUEUED_PREFIX "unmanaged"; + case NM_DEVICE_STATE_UNAVAILABLE: +- return "unavailable"; ++ return QUEUED_PREFIX "unavailable"; + case NM_DEVICE_STATE_DISCONNECTED: +- return "disconnected"; ++ return QUEUED_PREFIX "disconnected"; + case NM_DEVICE_STATE_PREPARE: +- return "prepare"; ++ return QUEUED_PREFIX "prepare"; + case NM_DEVICE_STATE_CONFIG: +- return "config"; ++ return QUEUED_PREFIX "config"; + case NM_DEVICE_STATE_NEED_AUTH: +- return "need-auth"; ++ return QUEUED_PREFIX "need-auth"; + case NM_DEVICE_STATE_IP_CONFIG: +- return "ip-config"; ++ return QUEUED_PREFIX "ip-config"; + case NM_DEVICE_STATE_IP_CHECK: +- return "ip-check"; ++ return QUEUED_PREFIX "ip-check"; + case NM_DEVICE_STATE_SECONDARIES: +- return "secondaries"; ++ return QUEUED_PREFIX "secondaries"; + case NM_DEVICE_STATE_ACTIVATED: +- return "activated"; ++ return QUEUED_PREFIX "activated"; + case NM_DEVICE_STATE_DEACTIVATING: +- return "deactivating"; ++ return QUEUED_PREFIX "deactivating"; + case NM_DEVICE_STATE_FAILED: +- return "failed"; ++ return QUEUED_PREFIX "failed"; + default: + break; + } +- return "unknown"; ++ return QUEUED_PREFIX "unknown"; ++} ++ ++static const char * ++state_to_string (NMDeviceState state) ++{ ++ return queued_state_to_string (state) + strlen (QUEUED_PREFIX); + } + + static const char * + reason_to_string (NMDeviceStateReason reason) + { + switch (reason) { + case NM_DEVICE_STATE_REASON_NONE: +@@ -6083,21 +6132,14 @@ reason_to_string (NMDeviceStateReason reason) + return "secondary-connection-failed"; + default: + break; + } + return "unknown"; + } + +-static inline gboolean +-state_implies_pending_action (NMDeviceState state) +-{ +- return ( state >= NM_DEVICE_STATE_PREPARE +- && state < NM_DEVICE_STATE_ACTIVATED); +-} +- + void + nm_device_state_changed (NMDevice *device, + NMDeviceState state, + NMDeviceStateReason reason) + { + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); + NMDeviceState old_state; +@@ -6131,18 +6173,14 @@ nm_device_state_changed (NMDevice *device, + return; + } + + old_state = priv->state; + priv->state = state; + priv->state_reason = reason; + +- if ( state_implies_pending_action (state) +- && !state_implies_pending_action (old_state)) +- nm_device_add_pending_action (device, "activation"); +- + nm_log_info (LOGD_DEVICE, "(%s): device state change: %s -> %s (reason '%s') [%d %d %d]", + nm_device_get_iface (device), + state_to_string (old_state), + state_to_string (state), + reason_to_string (reason), + old_state, + state, +@@ -6300,18 +6338,14 @@ nm_device_state_changed (NMDevice *device, + if (old_state == NM_DEVICE_STATE_ACTIVATED) + nm_dispatcher_call (DISPATCHER_ACTION_DOWN, nm_act_request_get_connection (req), device, NULL, NULL); + + /* Dispose of the cached activation request */ + if (req) + g_object_unref (req); + +- if ( state_implies_pending_action (old_state) +- && !state_implies_pending_action (state)) +- nm_device_remove_pending_action (device, "activation"); +- + priv->in_state_changed = FALSE; + } + + static gboolean + queued_set_state (gpointer user_data) + { + NMDevice *self = NM_DEVICE (user_data); +@@ -6330,15 +6364,15 @@ queued_set_state (gpointer user_data) + */ + priv->queued_state.id = 0; + new_state = priv->queued_state.state; + new_reason = priv->queued_state.reason; + nm_device_queued_state_clear (self); + + nm_device_state_changed (self, new_state, new_reason); +- nm_device_remove_pending_action (self, "queued state change"); ++ nm_device_remove_pending_action (self, queued_state_to_string (new_state)); + } else { + g_warn_if_fail (priv->queued_state.state == NM_DEVICE_STATE_UNKNOWN); + g_warn_if_fail (priv->queued_state.reason == NM_DEVICE_STATE_REASON_NONE); + } + return FALSE; + } + +@@ -6349,29 +6383,37 @@ nm_device_queue_state (NMDevice *self, + { + NMDevicePrivate *priv; + + g_return_if_fail (NM_IS_DEVICE (self)); + + priv = NM_DEVICE_GET_PRIVATE (self); + ++ /* "lock" the pending actions so that if there was a previously ++ * queued action that's about to be cleared, that doesn't drop ++ * the pending actions to 0 before we add the new pending action. ++ */ ++ nm_device_add_pending_action (self, "queued state lock"); ++ + /* We should only ever have one delayed state transition at a time */ + if (priv->queued_state.id) { + if (priv->queued_state.state == state) + return; + nm_log_warn (LOGD_DEVICE, "(%s): overwriting previously queued state change to %s (%s)", + nm_device_get_iface (self), + state_to_string (priv->queued_state.state), + reason_to_string (priv->queued_state.reason)); + nm_device_queued_state_clear (self); + } + + priv->queued_state.state = state; + priv->queued_state.reason = reason; + priv->queued_state.id = g_idle_add (queued_set_state, self); +- nm_device_add_pending_action (self, "queued state change"); ++ nm_device_add_pending_action (self, queued_state_to_string (state)); ++ ++ nm_device_remove_pending_action (self, "queued state lock"); + + nm_log_dbg (LOGD_DEVICE, "(%s): queued state change to %s due to %s (id %d)", + nm_device_get_iface (self), state_to_string (state), reason_to_string (reason), + priv->queued_state.id); + } + + NMDeviceState +@@ -6391,15 +6433,15 @@ nm_device_queued_state_clear (NMDevice *self) + { + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + if (priv->queued_state.id) { + nm_log_dbg (LOGD_DEVICE, "(%s): clearing queued state transition (id %d)", + nm_device_get_iface (self), priv->queued_state.id); + g_source_remove (priv->queued_state.id); +- nm_device_remove_pending_action (self, "queued state change"); ++ nm_device_remove_pending_action (self, queued_state_to_string (priv->queued_state.state)); + } + memset (&priv->queued_state, 0, sizeof (priv->queued_state)); + } + + NMDeviceState + nm_device_get_state (NMDevice *device) + { +@@ -7102,40 +7144,89 @@ nm_device_set_hw_addr (NMDevice *device, const guint8 *addr, + } + nm_device_bring_up (device, TRUE, NULL); + g_free (mac_str); + + return success; + } + ++/** ++ * nm_device_add_pending_action(): ++ * @device: the #NMDevice to add the pending action to ++ * @action: a static string that identifies the action ++ * ++ * Adds a pending action to the device. ++ */ + void + nm_device_add_pending_action (NMDevice *device, const char *action) + { + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); ++ GQuark qaction; ++ guint i; + +- priv->pending_actions++; +- nm_log_dbg (LOGD_DEVICE, "(%s): add_pending_action (%d): %s", +- nm_device_get_iface (device), priv->pending_actions, action); ++ qaction = g_quark_from_static_string (action); + +- if (priv->pending_actions == 1) ++ /* Shouldn't ever add the same pending action twice */ ++ for (i = 0; i < priv->pending_actions->len; i++) { ++ if (qaction == g_array_index (priv->pending_actions, GQuark, i)) { ++ nm_log_warn (LOGD_DEVICE, "(%s): add_pending_action (%d): '%s' already added", ++ nm_device_get_iface (device), ++ priv->pending_actions->len, ++ action); ++ g_warn_if_reached (); ++ return; ++ } ++ } ++ ++ g_array_prepend_val (priv->pending_actions, qaction); ++ nm_log_dbg (LOGD_DEVICE, "(%s): add_pending_action (%d): '%s'", ++ nm_device_get_iface (device), ++ priv->pending_actions->len, ++ action); ++ ++ if (priv->pending_actions->len == 1) + g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION); + } + ++/** ++ * nm_device_remove_pending_action(): ++ * @device: the #NMDevice to remove the pending action from ++ * @action: a static string that identifies the action ++ * ++ * Removes a pending action previously added by nm_device_add_pending_action(). ++ */ + void + nm_device_remove_pending_action (NMDevice *device, const char *action) + { + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); ++ GQuark qaction; ++ guint i; + +- priv->pending_actions--; +- nm_log_dbg (LOGD_DEVICE, "(%s): remove_pending_action (%d): %s", +- nm_device_get_iface (device), priv->pending_actions, action); ++ qaction = g_quark_from_static_string (action); + +- if (priv->pending_actions == 0) +- g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION); ++ /* Shouldn't ever add the same pending action twice */ ++ for (i = 0; i < priv->pending_actions->len; i++) { ++ if (qaction == g_array_index (priv->pending_actions, GQuark, i)) { ++ g_array_remove_index_fast (priv->pending_actions, i); ++ nm_log_dbg (LOGD_DEVICE, "(%s): remove_pending_action (%d): '%s'", ++ nm_device_get_iface (device), ++ priv->pending_actions->len, ++ action); ++ ++ if (priv->pending_actions->len == 0) ++ g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION); ++ return; ++ } ++ } ++ ++ nm_log_warn (LOGD_DEVICE, "(%s): remove_pending_action (%d): '%s' never added", ++ nm_device_get_iface (device), ++ priv->pending_actions->len, ++ action); + } + + gboolean + nm_device_has_pending_action (NMDevice *device) + { + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); + +- return priv->pending_actions > 0; ++ return priv->pending_actions->len > 0; + } +diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c +index 6a825bc..34f438c 100644 +--- a/src/nm-active-connection.c ++++ b/src/nm-active-connection.c +@@ -135,14 +135,20 @@ nm_active_connection_set_state (NMActiveConnection *self, + + if ( new_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED + || old_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { + nm_settings_connection_update_timestamp (NM_SETTINGS_CONNECTION (priv->connection), + (guint64) time (NULL), TRUE); + } + ++ if (priv->device) { ++ if ( old_state < NM_ACTIVE_CONNECTION_STATE_ACTIVATED ++ && new_state >= NM_ACTIVE_CONNECTION_STATE_ACTIVATED) ++ nm_device_remove_pending_action (priv->device, "activation"); ++ } ++ + if (priv->state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) { + /* Device is no longer relevant when deactivated */ + g_clear_object (&priv->device); + g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_DEVICES); + } + } + +@@ -262,8 +268,10 @@ set_property (GObject *object, guint pro + case PROP_INT_DEVICE: + g_warn_if_fail (priv->device == NULL); + priv->device = g_value_dup_object (value); +- if (priv->device) ++ if (priv->device) { + g_warn_if_fail (priv->device != priv->master); ++ nm_device_add_pending_action (priv->device, "activation"); ++ } + break; + case PROP_INT_USER_REQUESTED: + priv->user_requested = g_value_get_boolean (value); +-- +1.8.3.1 + +From 813ea5995bb5a35c115c46f30eefe18db2886afb Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Mon, 16 Dec 2013 16:52:36 +0100 +Subject: [PATCH 1/2] core: allow dynamic strings for pending action names + +Use a GSList of the string values, instead of an array of GQuarks. +Using GQuarks does not allow to add arbitrary strings, because they +would leak the internalized strings. The next patch will begin +using unique, non-const action strings. + +Given the rather small number of expected pending states, a singly +linked list seems appropriate. + +Signed-off-by: Thomas Haller + +(some fixes and simplifications by dcbw based on patch reviews) +--- + src/devices/nm-device.c | 49 +++++++++++++++++++++++++------------------------ + 1 file changed, 25 insertions(+), 24 deletions(-) + +diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c +index c1c7c69..e4644bf 100644 +--- a/src/devices/nm-device.c ++++ b/src/devices/nm-device.c +@@ -184,15 +184,15 @@ typedef struct { + gboolean initialized; + gboolean in_state_changed; + + NMDeviceState state; + NMDeviceStateReason state_reason; + QueuedState queued_state; + guint queued_ip_config_id; +- GArray *pending_actions; ++ GSList *pending_actions; + + char * udi; + char * path; + char * iface; /* may change, could be renamed by user */ + int ifindex; + gboolean is_software; + char * ip_iface; +@@ -391,15 +391,14 @@ nm_device_init (NMDevice *self) + priv->capabilities = NM_DEVICE_CAP_NM_SUPPORTED; + priv->state = NM_DEVICE_STATE_UNMANAGED; + priv->state_reason = NM_DEVICE_STATE_REASON_NONE; + priv->dhcp_timeout = 0; + priv->rfkill_type = RFKILL_TYPE_UNKNOWN; + priv->autoconnect = DEFAULT_AUTOCONNECT; + priv->available_connections = g_hash_table_new_full (g_direct_hash, g_direct_equal, g_object_unref, NULL); +- priv->pending_actions = g_array_sized_new (FALSE, TRUE, sizeof (GQuark), 5); + } + + static void + update_accept_ra_save (NMDevice *self) + { + NMDevicePrivate *priv; + const char *ip_iface; +@@ -5368,16 +5367,15 @@ finalize (GObject *object) + + if (priv->dhcp_manager) + g_object_unref (priv->dhcp_manager); + + if (priv->fw_manager) + g_object_unref (priv->fw_manager); + +- g_array_free (priv->pending_actions, TRUE); +- priv->pending_actions = NULL; ++ g_slist_free_full (priv->pending_actions, g_free); + + g_free (priv->udi); + g_free (priv->path); + g_free (priv->iface); + g_free (priv->ip_iface); + g_free (priv->driver); + g_free (priv->driver_version); +@@ -7153,78 +7152,80 @@ nm_device_set_hw_addr (NMDevice *device, const guint8 *addr, + * + * Adds a pending action to the device. + */ + void + nm_device_add_pending_action (NMDevice *device, const char *action) + { + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); +- GQuark qaction; +- guint i; ++ GSList *iter; ++ guint count; + +- qaction = g_quark_from_static_string (action); ++ g_return_if_fail (action); + + /* Shouldn't ever add the same pending action twice */ +- for (i = 0; i < priv->pending_actions->len; i++) { +- if (qaction == g_array_index (priv->pending_actions, GQuark, i)) { ++ for (iter = priv->pending_actions; iter; iter = iter->next) { ++ if (!strcmp (action, iter->data)) { + nm_log_warn (LOGD_DEVICE, "(%s): add_pending_action (%d): '%s' already added", + nm_device_get_iface (device), +- priv->pending_actions->len, ++ g_slist_length (priv->pending_actions), + action); +- g_warn_if_reached (); +- return; ++ g_return_val_if_reached (FALSE); + } + } + +- g_array_prepend_val (priv->pending_actions, qaction); ++ priv->pending_actions = g_slist_append (priv->pending_actions, g_strdup (action)); ++ count = g_slist_length (priv->pending_actions); ++ + nm_log_dbg (LOGD_DEVICE, "(%s): add_pending_action (%d): '%s'", + nm_device_get_iface (device), +- priv->pending_actions->len, ++ count, + action); + +- if (priv->pending_actions->len == 1) ++ if (count == 1) + g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION); + } + + /** + * nm_device_remove_pending_action(): + * @device: the #NMDevice to remove the pending action from + * @action: a static string that identifies the action + * + * Removes a pending action previously added by nm_device_add_pending_action(). + */ + void + nm_device_remove_pending_action (NMDevice *device, const char *action) + { + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); +- GQuark qaction; +- guint i; ++ GSList *iter; + +- qaction = g_quark_from_static_string (action); ++ g_return_if_fail (action); + + /* Shouldn't ever add the same pending action twice */ +- for (i = 0; i < priv->pending_actions->len; i++) { +- if (qaction == g_array_index (priv->pending_actions, GQuark, i)) { +- g_array_remove_index_fast (priv->pending_actions, i); ++ for (iter = priv->pending_actions; iter; iter = iter->next) { ++ if (!strcmp (action, iter->data)) { ++ g_free (iter->data); ++ priv->pending_actions = g_slist_delete_link (priv->pending_actions, iter); + nm_log_dbg (LOGD_DEVICE, "(%s): remove_pending_action (%d): '%s'", + nm_device_get_iface (device), +- priv->pending_actions->len, ++ g_slist_length (priv->pending_actions), + action); + +- if (priv->pending_actions->len == 0) ++ if (priv->pending_actions == NULL) + g_object_notify (G_OBJECT (device), NM_DEVICE_HAS_PENDING_ACTION); + return; + } + } + + nm_log_warn (LOGD_DEVICE, "(%s): remove_pending_action (%d): '%s' never added", + nm_device_get_iface (device), +- priv->pending_actions->len, ++ g_slist_length (priv->pending_actions), + action); ++ g_return_if_reached (); + } + + gboolean + nm_device_has_pending_action (NMDevice *device) + { + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); + +- return priv->pending_actions->len > 0; ++ return !!priv->pending_actions; + } +-- +1.8.3.1 + +From b6ef165cfefba44a496cc336e1f91bb93b28f3ff Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Mon, 16 Dec 2013 15:02:38 +0100 +Subject: [PATCH 2/2] core: fix NMActiveConnection to properly add/remove + pending action "activation" + +When a new activation request is received, NetworkManager creates a new +NMActiveConnection to track that request. The device may already be activated, +in which case NetworkManager stops the old activation and starts the new one, +but both exist in parallel for a short period of time. If the old +NMActiveConnection is activating and already has a pending 'activation' +action, when the new NMActiveConnection adds its own 'activating' action, +they will clash. This is fixed by making each NMActiveConnection's activation +pending action uniquely named. + +This fixes a g_warning with the following back trace: + + #0 0x000000328224edfd in g_logv () from /lib64/libglib-2.0.so.0 + #1 0x000000328224efe2 in g_log () from /lib64/libglib-2.0.so.0 + #2 0x000000328224f2e6 in g_warn_message () from /lib64/libglib-2.0.so.0 + #3 0x0000000000440aee in nm_device_add_pending_action (device=0x14002e0, action=0x50704a "activation") at devices/nm-device.c:7172 + #4 0x000000000047525c in nm_active_connection_set_device (self=0x141b3c0, device=0x14002e0) at nm-active-connection.c:364 + #5 0x0000000000475ec1 in set_property (object=0x141b3c0, prop_id=11, value=0x7fff7ff36c20, pspec=0x1405f70) at nm-active-connection.c:647 + #6 0x0000003282615d3e in g_object_newv () from /lib64/libgobject-2.0.so.0 + #7 0x00000032826162e6 in g_object_new_valist () from /lib64/libgobject-2.0.so.0 + #8 0x0000003282616654 in g_object_new () from /lib64/libgobject-2.0.so.0 + #9 0x0000000000474193 in nm_act_request_new (connection=0x13bb0e0, specific_object=0x0, subject=0x1447740, device=0x14002e0) at nm-activation-request.c:376 + #10 0x000000000048e477 in _new_active_connection (self=0x13e8060, connection=0x13bb0e0, specific_object=0x0, device=0x14002e0, subject=0x1447740, error=0x7fff7ff36f90) at nm-manager.c:2947 + #11 0x000000000048ed77 in impl_manager_activate_connection (self=0x13e8060, connection_path=0x134d590 "/org/freedesktop/NetworkManager/Settings/9", device_path=0x134d550 "/org/freedesktop/NetworkManager/Devices/1", + specific_object_path=0x0, context=0x143a9b0) at nm-manager.c:3206 + #12 0x00000000004876c8 in dbus_glib_marshal_nm_manager_VOID__BOXED_BOXED_BOXED_POINTER (closure=0x7fff7ff37220, return_value=0x0, n_param_values=5, param_values=0x1448830, invocation_hint=0x0, + marshal_data=0x48e9dd ) at nm-manager-glue.h:189 + #13 0x0000003284a0d6a9 in object_registration_message () from /lib64/libdbus-glib-1.so.2 + #14 0x000000348ea1ce66 in _dbus_object_tree_dispatch_and_unlock () from /lib64/libdbus-1.so.3 + #15 0x000000348ea0fa31 in dbus_connection_dispatch () from /lib64/libdbus-1.so.3 + #16 0x0000003284a0acc5 in message_queue_dispatch () from /lib64/libdbus-glib-1.so.2 + #17 0x0000003282247df6 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0 + #18 0x0000003282248148 in g_main_context_iterate.isra.22 () from /lib64/libglib-2.0.so.0 + #19 0x000000328224854a in g_main_loop_run () from /lib64/libglib-2.0.so.0 + #20 0x000000000042c6c0 in main (argc=1, argv=0x7fff7ff379b8) at main.c:629 + +Signed-off-by: Thomas Haller +--- + src/nm-active-connection.c | 17 ++++++++++++++--- + 1 file changed, 14 insertions(+), 3 deletions(-) + +diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c +index cb42417..a8a422c 100644 +--- a/src/nm-active-connection.c ++++ b/src/nm-active-connection.c +@@ -42,14 +42,16 @@ G_DEFINE_ABSTRACT_TYPE (NMActiveConnection, nm_active_connection, G_TYPE_OBJECT) + NMDevice *device; + + gboolean is_default; + gboolean is_default6; + NMActiveConnectionState state; + gboolean vpn; + ++ char *pending_activation_id; ++ + gboolean user_requested; + gulong user_uid; + NMDevice *master; + } NMActiveConnectionPrivate; + + enum { + PROP_0, +@@ -138,16 +140,20 @@ nm_active_connection_set_state (NMActiveConnection *self, + || old_state == NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { + nm_settings_connection_update_timestamp (NM_SETTINGS_CONNECTION (priv->connection), + (guint64) time (NULL), TRUE); + } + + if (priv->device) { + if ( old_state < NM_ACTIVE_CONNECTION_STATE_ACTIVATED +- && new_state >= NM_ACTIVE_CONNECTION_STATE_ACTIVATED) +- nm_device_remove_pending_action (priv->device, "activation"); ++ && new_state >= NM_ACTIVE_CONNECTION_STATE_ACTIVATED && ++ priv->pending_activation_id) ++ { ++ nm_device_remove_pending_action (priv->device, priv->pending_activation_id); ++ g_clear_pointer (&priv->pending_activation_id, g_free); ++ } + } + + if (priv->state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) { + /* Device is no longer relevant when deactivated */ + g_clear_object (&priv->device); + g_object_notify (G_OBJECT (self), NM_ACTIVE_CONNECTION_DEVICES); + } +@@ -357,15 +363,16 @@ set_property (GObject *object, guint prop_id, + priv->connection = g_value_dup_object (value); + break; + case PROP_INT_DEVICE: + g_warn_if_fail (priv->device == NULL); + priv->device = g_value_dup_object (value); + if (priv->device) { + g_warn_if_fail (priv->device != priv->master); +- nm_device_add_pending_action (priv->device, "activation"); ++ priv->pending_activation_id = g_strdup_printf ("activation::%p", (void *)object); ++ nm_device_add_pending_action (priv->device, priv->pending_activation_id); + } + break; + case PROP_INT_USER_REQUESTED: + priv->user_requested = g_value_get_boolean (value); + break; + case PROP_INT_USER_UID: + priv->user_uid = g_value_get_ulong (value); +@@ -732,14 +739,18 @@ static void + + g_free (priv->path); + priv->path = NULL; + g_free (priv->specific_object); + priv->specific_object = NULL; + + g_clear_object (&priv->connection); ++ if (priv->pending_activation_id) { ++ nm_device_remove_pending_action (priv->device, priv->pending_activation_id); ++ g_clear_pointer (&priv->pending_activation_id, g_free); ++ } + g_clear_object (&priv->device); + g_clear_object (&priv->master); + + G_OBJECT_CLASS (nm_active_connection_parent_class)->dispose (object); + } + + static void +-- +1.8.3.1 +