gdbus: Fix invalid memory access during interface removal
authorJohan Hedberg <johan.hedberg@intel.com>
Mon, 15 Oct 2012 10:21:11 +0000 (13:21 +0300)
committerMarcel Holtmann <marcel@holtmann.org>
Mon, 26 Nov 2012 13:44:48 +0000 (14:44 +0100)
If an interface is removed from the root path during the same mainloop
iteration that it was added we need to check for data->added before
doing the check for data->parent == NULL in the remove_interface()
function. Otherwise the added interface doesn't get removed from the
data->added list and will result in accessing freed memory:

==337== Invalid read of size 8
==337==    at 0x4F65AFA: dbus_message_iter_append_basic (in /usr/lib64/libdbus-1.so.3.7.1)
==337==    by 0x1247B5: append_interface (object.c:556)
==337==    by 0x4C8DC5C: g_slist_foreach (gslist.c:840)
==337==    by 0x1261F7: process_changes (object.c:594)
==337==    by 0x126372: generic_unregister (object.c:997)
==337==    by 0x4F69669: ??? (in /usr/lib64/libdbus-1.so.3.7.1)
==337==    by 0x4F5CE51: dbus_connection_unregister_object_path (in /usr/lib64/libdbus-1.so.3.7.1)
==337==    by 0x125E81: object_path_unref (object.c:1236)
==337==    by 0x126136: g_dbus_unregister_interface (object.c:1361)
==337==    by 0x14CDF0: service_exit (service.c:581)
==337==    by 0x177556: plugin_cleanup (plugin.c:242)
==337==    by 0x12221F: main (main.c:559)
==337==  Address 0x5bc1550 is 0 bytes inside a block of size 56 free'd
==337==    at 0x4A079AE: free (vg_replace_malloc.c:427)
==337==    by 0x4C7850E: g_free (gmem.c:252)
==337==    by 0x125DB0: remove_interface (object.c:671)
==337==    by 0x125E3B: object_path_unref (object.c:1230)
==337==    by 0x126136: g_dbus_unregister_interface (object.c:1361)
==337==    by 0x14CDF0: service_exit (service.c:581)
==337==    by 0x177556: plugin_cleanup (plugin.c:242)
==337==    by 0x12221F: main (main.c:559)

gdbus/object.c

index aa38c07..4147361 100644 (file)
@@ -655,12 +655,6 @@ static gboolean remove_interface(struct generic_data *data, const char *name)
                iface->user_data = NULL;
        }
 
-       if (data->parent == NULL) {
-               g_free(iface->name);
-               g_free(iface);
-               return TRUE;
-       }
-
        /*
         * Interface being removed was just added, on the same mainloop
         * iteration? Don't send any signal
@@ -672,6 +666,12 @@ static gboolean remove_interface(struct generic_data *data, const char *name)
                return TRUE;
        }
 
+       if (data->parent == NULL) {
+               g_free(iface->name);
+               g_free(iface);
+               return TRUE;
+       }
+
        data->removed = g_slist_prepend(data->removed, iface->name);
        g_free(iface);