rpms/NetworkManager
5
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix a couple bugs (rh #1034921) (rh #1029213) (rh #1032819)

Browse Source 
- 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)
This commit is contained in:
Dan Williams 2013-12-18 13:03:30 -06:00
parent 7212c621a9
commit 170f1c14b7
4 changed files with 1029 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
NetworkManager.spec
View File

@ -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 <dcbw@redhat.com> - 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 <danw@redhat.com> - 0.9.9.0-20.git20131003
- core: Fix PtP/peer address support, for OpenVPN (rh #1018317)

98
rh1029213-ignore-RA-default-routes.patch Normal file
View File

@ -0,0 +1,98 @@
From 8586353b09460ec0a619058421743dd7d424a75d Mon Sep 17 00:00:00 2001
From: Dan Williams <dcbw@redhat.com>
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 <dcbw@redhat.com>
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

86
rh1032819-set-broadcast-address.patch Normal file
View File

@ -0,0 +1,86 @@
From 7eb12a5b21f87d7592ec2c5235d1ed90c4fac132 Mon Sep 17 00:00:00 2001
From: Dan Williams <dcbw@redhat.com>
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

833
rh1034921-startup-link-wait.patch Normal file
View File

@ -0,0 +1,833 @@
From 07281f21a71fa333288821ad863accfeedfff567 Mon Sep 17 00:00:00 2001
From: Dan Williams <dcbw@redhat.com>
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 <thaller@redhat.com>
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 <thaller@redhat.com>
(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 <thaller@redhat.com>
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 <impl_manager_activate_connection>) 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 <thaller@redhat.com>
---
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