Re: Accessing previously freed data.

[Drew Moseley <dmoseley mvista com>]

Dan Williams wrote:
> I looked over the code there again, and it's correct AFAICT.
> g_slist_delete_link() in the sync_devices() function should be doing
> what its intended to do; it removes one link from the linked list
> priv->devices and frees the link.  It doesn't free the data of course,
> but that should get correctly unref-ed right below.  And after that, the
> device should no longer be in priv->devices at all.  That doesn't mean
> there isn't a refcounting bug, but I'm still not quite sure what's
> causing the invalid access later on...

Hi Dan,

Looks like the issue is that the delete_link call is incorrect since the node being passed in is from the copy of the list.  The patch below addresses this by building the new list by adding the items that exist and then deleting the old list.

Thoughts?

Drew


diff --git a/src/nm-manager.c b/src/nm-manager.c
index 30660f1..9a8c1e8 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1479,12 +1479,11 @@ static void
 sync_devices (NMManager *self)
 {
 	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
-	GSList *devices;
+	GSList *devices = NULL;
 	GSList *iter;
 
 	/* Remove devices which are no longer known to HAL */
-	devices = g_slist_copy (priv->devices);
-	for (iter = devices; iter; iter = iter->next) {
+	for (iter = priv->devices; iter; iter = iter->next) {
 		NMDevice *device = NM_DEVICE (iter->data);
 		const char *udi = nm_device_get_udi (device);
 
@@ -1493,13 +1492,14 @@ sync_devices (NMManager *self)
 				nm_device_set_managed (device, TRUE, NM_DEVICE_STATE_REASON_NOW_MANAGED);
 			else
 				nm_device_set_managed (device, FALSE, NM_DEVICE_STATE_REASON_NOW_UNMANAGED);
+			devices = g_slist_prepend(devices, device);
 		} else {
-			priv->devices = g_slist_delete_link (priv->devices, iter);
 			remove_one_device (self, device);
 		}
 	}
 
-	g_slist_free (devices);
+	g_slist_free (priv->devices);
+	priv->devices = devices;
 
 	/* Get any new ones */
 	nm_hal_manager_query_devices (priv->hal_mgr);