GNOME Bugzilla – Bug 708529
xdgmime: valgrind warns about invalid reads
Last modified: 2013-10-04 21:53:18 UTC
Opening the file chooser from glade: ==21461== Invalid read of size 1 ==21461== at 0x4D378C2: __gio_xdg_cache_mime_type_subclass (xdgmimecache.c:848) ==21461== by 0x4CDF1BC: g_content_type_is_a (gcontenttype.c:158) ==21461== by 0x34D8031E95: gtk_recent_filter_filter (gtkrecentfilter.c:733) ==21461== by 0x34D802F167: _gtk_recent_chooser_get_items (gtkrecentchooserutils.c:387) ==21461== by 0x34D802D07F: idle_populate_func (gtkrecentchoosermenu.c:1011) ==21461== by 0x34D7A20477: gdk_threads_dispatch (gdk.c:804) ==21461== by 0x544FFC5: g_main_context_dispatch (gmain.c:3065) ==21461== by 0x5450317: g_main_context_iterate.isra.23 (gmain.c:3712) ==21461== by 0x54503BB: g_main_context_iteration (gmain.c:3773) ==21461== by 0x34D7FC2AF4: gtk_main_iteration (gtkmain.c:1262) ==21461== by 0x408EB4: main (in /usr/bin/glade) ==21461== Address 0x5b7828e is 2 bytes before a block of size 1 alloc'd ==21461== at 0x4A06409: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==21461== by 0x54559EE: g_malloc (gmem.c:104) ==21461== by 0x546CE2E: g_strdup (gstrfuncs.c:364) ==21461== by 0x34D80317D6: gtk_recent_filter_add_mime_type (gtkrecentfilter.c:518) ==21461== by 0x34D8031A4A: parser_end_element (gtkrecentfilter.c:310) ==21461== by 0x34D7EDCE65: end_element (gtkbuilderparser.c:852) ==21461== by 0x5453FC8: g_markup_parse_context_parse (gmarkup.c:1602) ==21461== by 0x34D7EDD54D: _gtk_builder_parser_parse_buffer (gtkbuilderparser.c:1285) ==21461== by 0x34D7ED8D17: _gtk_builder_extend_with_template (gtkbuilder.c:1139) ==21461== by 0x34D811AB6F: gtk_widget_init_template (gtkwidget.c:15748) ==21461== by 0x4FE3ADA: g_type_create_instance (gtype.c:1868) ==21461== by 0x4FC7344: g_object_new_internal (gobject.c:1746)
Created attachment 255477 [details] [review] xdgmime: Fix an invalid read This commit factors out a function for comparing string suffixes, and at the same time makes it safe for mime types that are shorter than the "/*" suffix. ==25418== Invalid read of size 1 ==25418== at 0x3C6D0F9D22: __gio_xdg_cache_mime_type_subclass (xdgmimecache.c:848) ==25418== by 0x3C6D09ED8C: g_content_type_is_a (gcontenttype.c:158) ==25418== by 0x34D8031E95: gtk_recent_filter_filter (gtkrecentfilter.c:733) ==25418== by 0x34D802F167: _gtk_recent_chooser_get_items (gtkrecentchooserutils.c:387) ==25418== by 0x34D802D07F: idle_populate_func (gtkrecentchoosermenu.c:1011) ==25418== by 0x34D7A20477: gdk_threads_dispatch (gdk.c:804) ==25418== by 0x3C6C0492F5: g_main_context_dispatch (gmain.c:3065) ==25418== by 0x3C6C049677: g_main_context_iterate.isra.23 (gmain.c:3712) ==25418== by 0x3C6C04972B: g_main_context_iteration (gmain.c:3773) ==25418== by 0x34D7FC2AF4: gtk_main_iteration (gtkmain.c:1262) ==25418== by 0x408EB4: main (in /usr/bin/glade)
I'll do the same fix against freedesktop.org xdgmime if it looks good.
Review of attachment 255477 [details] [review]: This looks good to me.
Maybe it would have been easier to just add the check for length<2 instead of making this generic (and slower)?
Sure, that would be another option. Are you concerned about the function call overhead here? For what it's worth, an optimizing compiler should be able to inline this: the compiler sees the function definition and it's a static function and only visible within one compilation unit.
I don't strongly care either way. If you don't mind doing the patch the simple way, then I think that's better. If you'd prefer to commit this one because you think it's cleaner, then I'm OK with that too :)
OK, thanks. I went with the generic patch as it seemed slightly easier to read, at least for me. Attachment 255477 [details] pushed as be7f401 - xdgmime: Fix an invalid read