summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAleksander Morgado <aleksander@aleksander.es>2017-10-05 11:39:36 +0200
committerAleksander Morgado <aleksander@aleksander.es>2017-10-05 11:43:21 +0200
commitb5c3120181492a731654f081795367ace556412f (patch)
treec992d2ba051d9710927bcd739b76e2c542fd5203
parentb69326c14b905fdbfc04cfb41b78cd4f2d09c2ac (diff)
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..
-rw-r--r--src/libqmi-glib/qmi-device.c6
1 files changed, 4 insertions, 2 deletions
diff --git a/src/libqmi-glib/qmi-device.c b/src/libqmi-glib/qmi-device.c
index 40ad925..96b11cf 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);