After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 569642 - g_icon_to_string_tokenized() invalid read valgrind error
g_icon_to_string_tokenized() invalid read valgrind error
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: daemon
1.1.x
Other Linux
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2009-01-29 10:18 UTC by Sebastien Bacher
Modified: 2009-03-02 09:26 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26


Attachments
Fixes double free (439 bytes, patch)
2009-01-29 12:26 UTC, Alexander Larsson
none Details | Review

Description Sebastien Bacher 2009-01-29 10:18:49 UTC
the bug has been described on https://bugs.launchpad.net/bugs/320386

"Crash while navigating photos on iPhone

2.19.5-0ubuntu1"

valgrind error log 

"==5763== Invalid read of size 1
==5763==    at 0x4027443: strlen (mc_replace_strmem.c:242)
==5763==    by 0x4184C23: g_string_append_uri_escaped (gstring.c:770)
==5763==    by 0x4093BA9: g_icon_to_string_tokenized (gicon.c:203)
==5763==    by 0x4093EF8: g_icon_to_string (gicon.c:291)
==5763==    by 0x404FDAF: append_object (gvfsdaemonprotocol.c:66)
==5763==    by 0x40500E8: _g_dbus_append_file_attribute (gvfsdaemonprotocol.c:207)
==5763==    by 0x405028A: _g_dbus_append_file_info (gvfsdaemonprotocol.c:266)
==5763==    by 0x805F676: create_reply (gvfsjobqueryinfo.c:197)
==5763==    by 0x805B0C6: send_reply (gvfsjobdbus.c:166)
==5763==    by 0x4100340: g_cclosure_marshal_VOID__VOID (gmarshal.c:77)
==5763==    by 0x40E695C: g_type_class_meta_marshal (gclosure.c:878)
==5763==    by 0x40E6644: g_closure_invoke (gclosure.c:767)
==5763==    by 0x40FFA28: signal_emit_unlocked_R (gsignal.c:3282)
==5763==    by 0x40FE93E: g_signal_emit_valist (gsignal.c:2977)
==5763==    by 0x40FEC51: g_signal_emit (gsignal.c:3034)
==5763==    by 0x805A738: g_vfs_job_send_reply (gvfsjob.c:237)
==5763==    by 0x805A8E0: g_vfs_job_succeeded (gvfsjob.c:303)
==5763==    by 0x8052189: try_query_info (gvfsbackendgphoto2.c:1867)
==5763==    by 0x805F5DD: try (gvfsjobqueryinfo.c:170)
==5763==    by 0x805A679: g_vfs_job_try (gvfsjob.c:216)
==5763==    by 0x805683F: g_vfs_daemon_queue_job (gvfsdaemon.c:453)
==5763==    by 0x8056265: job_source_new_job_callback (gvfsdaemon.c:320)
==5763==    by 0x4100F75: g_cclosure_marshal_VOID__OBJECT (gmarshal.c:636)
==5763==    by 0x40E6644: g_closure_invoke (gclosure.c:767)
==5763==    by 0x40FF7E5: signal_emit_unlocked_R (gsignal.c:3244)
==5763==    by 0x40FE93E: g_signal_emit_valist (gsignal.c:2977)
==5763==    by 0x40FEC51: g_signal_emit (gsignal.c:3034)
==5763==    by 0x805AB20: g_vfs_job_source_new_job (gvfsjobsource.c:100)
==5763==    by 0x8058BAC: backend_dbus_handler (gvfsbackend.c:587)
==5763==    by 0x8057649: daemon_message_func (gvfsdaemon.c:981)
==5763==    by 0x42973B8: dbus_connection_dispatch (dbus-connection.c:4406)
==5763==    by 0x8064E97: message_queue_dispatch (dbus-gmain.c:127)
==5763==    by 0x415CAAD: g_main_dispatch (gmain.c:1814)
==5763==    by 0x415DF85: g_main_context_dispatch (gmain.c:2367)
==5763==    by 0x415E4D9: g_main_context_iterate (gmain.c:2448)
==5763==    by 0x415ECB0: g_main_loop_run (gmain.c:2656)
==5763==    by 0x8055A13: daemon_main (daemon-main.c:270)
==5763==    by 0x8055A7B: main (daemon-main-generic.c:39)
==5763==  Address 0x5453b01 is 1 bytes inside a block of size 51 free'd
==5763==    at 0x4025B4A: free (vg_replace_malloc.c:323)
==5763==    by 0x4165726: g_free (gmem.c:190)
==5763==    by 0x4093BB4: g_icon_to_string_tokenized (gicon.c:206)
==5763==    by 0x4093EF8: g_icon_to_string (gicon.c:291)
==5763==    by 0x404FDAF: append_object (gvfsdaemonprotocol.c:66)
==5763==    by 0x40500E8: _g_dbus_append_file_attribute (gvfsdaemonprotocol.c:207)
==5763==    by 0x405028A: _g_dbus_append_file_info (gvfsdaemonprotocol.c:266)
==5763==    by 0x805F676: create_reply (gvfsjobqueryinfo.c:197)
==5763==    by 0x805B0C6: send_reply (gvfsjobdbus.c:166)
==5763==    by 0x4100340: g_cclosure_marshal_VOID__VOID (gmarshal.c:77)
==5763==    by 0x40E695C: g_type_class_meta_marshal (gclosure.c:878)
==5763==    by 0x40E6644: g_closure_invoke (gclosure.c:767)
==5763==    by 0x40FFA28: signal_emit_unlocked_R (gsignal.c:3282)
==5763==    by 0x40FE93E: g_signal_emit_valist (gsignal.c:2977)
==5763==    by 0x40FEC51: g_signal_emit (gsignal.c:3034)
==5763==    by 0x805A738: g_vfs_job_send_reply (gvfsjob.c:237)
==5763==    by 0x805A8E0: g_vfs_job_succeeded (gvfsjob.c:303)
==5763==    by 0x8052189: try_query_info (gvfsbackendgphoto2.c:1867)
==5763==    by 0x805F5DD: try (gvfsjobqueryinfo.c:170)
==5763==    by 0x805A679: g_vfs_job_try (gvfsjob.c:216)
==5763==    by 0x805683F: g_vfs_daemon_queue_job (gvfsdaemon.c:453)
==5763==    by 0x8056265: job_source_new_job_callback (gvfsdaemon.c:320)
==5763==    by 0x4100F75: g_cclosure_marshal_VOID__OBJECT (gmarshal.c:636)
==5763==    by 0x40E6644: g_closure_invoke (gclosure.c:767)
==5763==    by 0x40FF7E5: signal_emit_unlocked_R (gsignal.c:3244)
==5763==    by 0x40FE93E: g_signal_emit_valist (gsignal.c:2977)
==5763==    by 0x40FEC51: g_signal_emit (gsignal.c:3034)
==5763==    by 0x805AB20: g_vfs_job_source_new_job (gvfsjobsource.c:100)
==5763==    by 0x8058BAC: backend_dbus_handler (gvfsbackend.c:587)
==5763==    by 0x8057649: daemon_message_func (gvfsdaemon.c:981)
==5763==    by 0x42973B8: dbus_connection_dispatch (dbus-connection.c:4406)
==5763==    by 0x8064E97: message_queue_dispatch (dbus-gmain.c:127)
==5763==    by 0x415CAAD: g_main_dispatch (gmain.c:1814)
==5763==    by 0x415DF85: g_main_context_dispatch (gmain.c:2367)
==5763==    by 0x415E4D9: g_main_context_iterate (gmain.c:2448)
==5763==    by 0x415ECB0: g_main_loop_run (gmain.c:2656)
==5763==    by 0x8055A13: daemon_main (daemon-main.c:270)
==5763==    by 0x8055A7B: main (daemon-main-generic.c:39)"
Comment 1 Sebastien Bacher 2009-01-29 10:29:10 UTC
the corresponding call

"  for (i = 0; i < tokens->len; i++)
    {
      char *token;

      token = g_ptr_array_index (tokens, i);

      g_string_append_c (s, ' ');
      /* We really only need to escape spaces here, so allow lots of otherwise reserved chars */
      g_string_append_uri_escaped (s, token,
				   G_URI_RESERVED_CHARS_ALLOWED_IN_PATH, TRUE);

      g_free (token);
    }"

desrt added the g_free() call on http://svn.gnome.org/viewvc/glib?view=revision&revision=7743 ccing him now, it should not be called on each iteration
Comment 2 Sebastien Bacher 2009-01-29 10:30:10 UTC
something around those lines should be working

--- gicon.c.gnome	2009-01-29 11:21:24.000000000 +0100
+++ gicon.c	2009-01-29 11:21:55.000000000 +0100
@@ -203,6 +203,7 @@
       g_string_append_uri_escaped (s, token,
 				   G_URI_RESERVED_CHARS_ALLOWED_IN_PATH, TRUE);
 
+    if (i == tokens->len - 1)
       g_free (token);
     }
Comment 3 Sebastien Bacher 2009-01-29 10:46:09 UTC
you can ignore the previous comment I read the code too quickly, the g_free() call is correct there, could be that the array has an element twice?
Comment 4 Alexander Larsson 2009-01-29 12:26:40 UTC
Created attachment 127452 [details] [review]
Fixes double free

Can you test this patch? It fixes a similar bug in the same area, but doesn't *obviously* match the valgrind messages.
Comment 5 Alexander Larsson 2009-01-29 12:29:44 UTC
(The fix is commited to svn btw)
Comment 6 Sebastien Bacher 2009-01-29 15:59:53 UTC
the submitter confirms that the fix is working