libqmi-glib,device: fix invalid memory read on source destruction

When the input source was created, we were explicitly removing the
original reference once the source was attached to the main context,
which meant that the context was then owner of the source.

If the source callback was called and G_SOURCE_REMOVE executed, the
main context would then be destroying the last reference of the
GSource. Any further check on the GSource from our logic, like the
g_source_destroy() call we were doing would result in an invalid
memory read as reported by valgrind:

    ==26546== Invalid read of size 8
    ==26546==    at 0x66CE505: g_source_destroy (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x5391CF9: destroy_iostream (qmi-device.c:2403)
    ==26546==    by 0x5391FE3: qmi_device_close_async (qmi-device.c:2463)
    ==26546==    by 0x53920BA: qmi_device_close (qmi-device.c:2474)
    ==26546==    by 0x1F96D2: port_open_step (mm-port-qmi.c:517)
    ==26546==    by 0x1F8F2F: qmi_device_open_first_ready (mm-port-qmi.c:307)
    ==26546==    by 0x6122D52: ??? (in /usr/lib/libgio-2.0.so.0.5200.3)
    ==26546==    by 0x6123775: ??? (in /usr/lib/libgio-2.0.so.0.5200.3)
    ==26546==    by 0x5390F24: internal_proxy_open_ready (qmi-device.c:2007)
    ==26546==    by 0x6122D52: ??? (in /usr/lib/libgio-2.0.so.0.5200.3)
    ==26546==    by 0x6123775: ??? (in /usr/lib/libgio-2.0.so.0.5200.3)
    ==26546==    by 0x53AA1A4: internal_proxy_open_ready (qmi-ctl.c:4095)
    ==26546==  Address 0xf8fb700 is 32 bytes inside a block of size 104 free'd
    ==26546==    at 0x4C2E14B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==26546==    by 0x66CD988: ??? (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x66D09C7: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x66D0C87: ??? (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x66D0FA1: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x144A3B: main (main.c:180)
    ==26546==  Block was alloc'd at
    ==26546==    at 0x4C2EF35: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==26546==    by 0x66D6080: g_malloc0 (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x66CE3AD: g_source_new (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x610A2A7: g_pollable_source_new (in /usr/lib/libgio-2.0.so.0.5200.3)
    ==26546==    by 0x611C08E: ??? (in /usr/lib/libgio-2.0.so.0.5200.3)
    ==26546==    by 0x539027F: setup_iostream (qmi-device.c:1617)
    ==26546==    by 0x53907D2: create_iostream_with_socket (qmi-device.c:1749)
    ==26546==    by 0x5390405: wait_for_proxy_cb (qmi-device.c:1662)
    ==26546==    by 0x66D1342: ??? (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x66D08C4: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x66D0C87: ??? (in /usr/lib/libglib-2.0.so.0.5200.3)
    ==26546==    by 0x66D0FA1: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.5200.3)

Avoid this by making sure we always have a valid GSource reference so
that we can can safely call g_source_destroy() on valid memory..

1 files changed, 4 insertions, 2 deletions

diff --git a/src/libqmi-glib/qmi-device.c b/src/libqmi-glib/qmi-device.c
index 40ad9255..96b11cfc 100644
--- a/src/libqmi-glib/qmi-device.c
+++ b/src/libqmi-glib/qmi-device.c
@@ -1623,7 +1623,6 @@ setup_iostream (GTask *task)
                            self,
                            NULL);
     g_source_attach (self->priv->input_source, g_main_context_get_thread_default ());
-    g_source_unref (self->priv->input_source);
 
     g_task_return_boolean (task, TRUE);
     g_object_unref (task);
@@ -2407,7 +2406,10 @@ qmi_device_open (QmiDevice *self,
 static void
 destroy_iostream (QmiDevice *self)
 {
-    g_clear_pointer (&self->priv->input_source, g_source_destroy);
+    if (self->priv->input_source) {
+        g_source_destroy (self->priv->input_source);
+        g_clear_pointer (&self->priv->input_source, g_source_unref);
+    }
     g_clear_pointer (&self->priv->buffer, g_byte_array_unref);
     g_clear_object (&self->priv->istream);
     g_clear_object (&self->priv->ostream);