wifi: prevent infinite loop when disposing of supplicant proxy (rh #538717)
authorDan Williams <dcbw@redhat.com>
Wed, 14 Apr 2010 22:16:48 +0000 (15:16 -0700)
committerDan Williams <dcbw@redhat.com>
Wed, 14 Apr 2010 22:16:48 +0000 (15:16 -0700)
This has been around a long time, but is very hard to trigger.  It appears
to happen mostly if the supplicant segfaults on resume but has been seen
in other cases as well.

For whatever reason, the DBusGProxy's refcount reaches 0 and the proxy gets
disposed of.  That in turn disposes of all the pending calls that are
in-progress on the proxy.  Since we give the pending calls a closure, that
closure (nm_supplicant_info_destroy) gets called when the pending calls
are destroyed.  That closure unrefs the proxy again.

Since DBusGProxy doesn't have any protection in its dispose() handler
against re-entrant disposes (which is arguably a bug of the client)
we end up infinite looping in nm_supplicant_info_destroy().

Fix that by ensuring we return early if we detect that we are already
freeing the NMSupplicantInfo object, and thus don't try to dispose
of the proxy yet again.

src/supplicant-manager/nm-supplicant-interface.c

index da1a856..b91a509 100644 (file)
@@ -144,6 +144,7 @@ typedef struct {
        DBusGProxy *proxy;
        NMCallStore *store;
        DBusGProxyCall *call;
+       gboolean disposing;
 } NMSupplicantInfo;
 
 static NMSupplicantInfo *
@@ -164,10 +165,11 @@ nm_supplicant_info_new (NMSupplicantInterface *interface,
 static void
 nm_supplicant_info_set_call (NMSupplicantInfo *info, DBusGProxyCall *call)
 {
-       if (call) {
-               nm_call_store_add (info->store, G_OBJECT (info->proxy), (gpointer) call);
-               info->call = call;
-       }
+       g_return_if_fail (info != NULL);
+       g_return_if_fail (call != NULL);
+
+       nm_call_store_add (info->store, G_OBJECT (info->proxy), (gpointer) call);
+       info->call = call;
 }
 
 static void
@@ -175,13 +177,25 @@ nm_supplicant_info_destroy (gpointer user_data)
 {
        NMSupplicantInfo *info = (NMSupplicantInfo *) user_data;
 
-       if (info->call)
-               nm_call_store_remove (info->store, G_OBJECT (info->proxy), info->call);
+       /* Guard against double-disposal; since DBusGProxy doesn't guard against
+        * double-disposal, we could infinite loop here if we're in the middle of
+        * some wpa_supplicant D-Bus calls.  When the supplicant dies we'll dispose
+        * of the proxy, which kills all its pending calls, which brings us here.
+        * Then when we unref the proxy here, its dispose() function will get called
+        * again, and we get right back here until we segfault because our callstack
+        * is too long.
+        */
+       if (!info->disposing) {
+               info->disposing = TRUE;
 
-       g_object_unref (info->proxy);
-       g_object_unref (info->interface);
+               if (info->call)
+                       nm_call_store_remove (info->store, G_OBJECT (info->proxy), info->call);
 
-       g_slice_free (NMSupplicantInfo, info);
+               g_object_unref (info->proxy);
+               g_object_unref (info->interface);
+
+               g_slice_free (NMSupplicantInfo, info);
+       }
 }