Bug 6824 - Crash if MIME data changes during a call to xdg_mime_get_mime_type_for_file()
Summary: Crash if MIME data changes during a call to xdg_mime_get_mime_type_for_file()
Status: RESOLVED FIXED
Alias: None
Product: xdgmime
Classification: Unclassified
Component: xdgmime (show other bugs)
Version: unspecified
Hardware: x86 (IA32) Linux (All)
: high critical
Assignee: Jonathan Blandford
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-03 17:47 UTC by Tom Hughes
Modified: 2007-06-01 09:13 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
possible patch (3.38 KB, patch)
2006-05-18 07:12 UTC, Joe Shaw
Details | Splinter Review
Fix a cut-and-paste error from the previous patch. (3.50 KB, patch)
2006-05-18 07:14 UTC, Joe Shaw
Details | Splinter Review
The last patch didn't quite fix things, this one does. (4.74 KB, patch)
2006-05-24 00:16 UTC, Joe Shaw
Details | Splinter Review

Description Tom Hughes 2006-05-03 17:47:32 UTC
A segmentation fault can occur if the shared MIME data changes on disk at the
wrong moment.

The problem is that _xdg_mime_magic_lookup_data() is iterating over the list of
matchers given it by xdg_mime_get_mime_type_for_file() (ie the global_match
list) and calls xdg_mime_mime_type_equal() which calls xdg_mime_init() which may
decide that the MIME data has changed in which case it discards the global_match
list and rebuilds it.

When we return to xdg_mime_get_mime_type_for_file() the list that was being
interated over has been freed and we crash shortly thereafter.

Of course you would normally have to ve very unlucky to encounter this as the
MIME data doesn't change very often, but there is a second bug which can cause
xdgmime to decide to reread it when it hasn't changed - in fact after every
check (every 5 seconds) it is reread.

The reason for this is that if HAVE_MMAP is not defined
_xdg_mime_cache_new_from_file() always returns NULL which means that
xdg_mime_init_from_directory() does not add mime.cache to the dir_time_list.

When xdg_check_dirs() is next called to check for changes it fails to find
anything to compare mime.cache to so forces a reload. This continues ad infinitum.

This problem was encountered in beagle and is reported there as Gnome bug
#339815 (http://bugzilla.gnome.org/show_bug.cgi?id=339815) which is itself an
upstream reporting of Fedora bug #189964
(https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189964).
Comment 1 Joe Shaw 2006-05-18 07:11:24 UTC
Bumping this up to critical.

I'm going to attach a patch which I think will work around the problem.  It's
not particularly elegant, but it might be the way to go.  It simply creates
private versions of the two public methods called from
_xdg_mime_magic_lookup_data() so that xdg_mime_init() isn't called.
Comment 2 Joe Shaw 2006-05-18 07:12:12 UTC
Created attachment 5670 [details] [review]
possible patch
Comment 3 Joe Shaw 2006-05-18 07:14:32 UTC
Created attachment 5671 [details] [review]
Fix a cut-and-paste error from the previous patch.

Oops, forgot to actuall remove the call to xdg_mime_init() in
_xdg_mime_mime_type_equal().
Comment 4 Joe Shaw 2006-05-24 00:16:11 UTC
Created attachment 5721 [details] [review]
The last patch didn't quite fix things, this one does.

This patch fixes the problem for me on various systems I've tested with Beagle.
 While I think this patch is probably the right thing either way, there may be
another bug here that this fix would cover up, which is that the MIME data
appears to be reloaded even if it hasn't been updated.
Comment 5 Tom Hughes 2006-05-24 00:28:16 UTC
The reloading when the MIME data hasn't been updated is the second problem I
covered in my original beagle ticket
(http://bugzilla.gnome.org/show_bug.cgi?id=339815) - it is caused by beagle not
defining HAVE_MMAP which I think has now been fixed.

It is arguably a bug that if xdgmime behaves like that if HAVE_MMAP is not
defined though.
Comment 6 Joe Shaw 2006-05-24 00:56:42 UTC
Tom, yes, I fixed the HAVE_MMAP issue upstream.  I had applied a patch to our
local package which should have fixed it, but it appears to not have been taken.
 That's why we continued to see the issue in our SUSE packages.  Oops. :)
Comment 7 Matthias Clasen 2006-07-19 20:27:37 UTC
The patch makes sense to me; we shouldn't call init when using xdg functions
internally, in order to not mess up the data structures we are using.

I'm applying the patch to the GTK+ copy of xdgmime. I'd also apply it
to cvs xdgmime, but my checkout is broken.
Comment 8 Christian - Manny Calavera - Neumair 2007-06-01 09:13:04 UTC
Thanks, I just committed the patch with a slight modification: I also changed the recursive xdg_mime_mime_type_subclass() call in _xdg_mime_mime_type_subclass() to the private version.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.