Bug 4408 - metacity segfaults in oxygen theme (gtk2)
Summary: metacity segfaults in oxygen theme (gtk2)
Status: RESOLVED FIXED
Alias: None
Product: Mageia
Classification: Unclassified
Component: RPM Packages (show other bugs)
Version: Cauldron
Hardware: x86_64 Linux
Priority: Normal major
Target Milestone: ---
Assignee: Juan Luis Baptiste
QA Contact:
URL:
Whiteboard:
Keywords: PATCH, UPSTREAM
Depends on:
Blocks:
 
Reported: 2012-02-04 19:05 CET by Thierry Vignaud
Modified: 2012-02-10 19:34 CET (History)
2 users (show)

See Also:
Source RPM: oxygen-gtk-1.2.0-2.mga2.src.rpm
CVE:
Status comment:

Attachments
GDB trace (5.02 KB, text/plain)
2012-02-04 19:07 CET, Thierry Vignaud 		Details
patch (for testing) (776 bytes, patch)
2012-02-05 15:27 CET, Hugo Pereira Da Costa 		Details | Diff
GDB trace with path applied (3.99 KB, text/plain)
2012-02-05 17:40 CET, Thierry Vignaud 		Details
another patch (870 bytes, patch)
2012-02-05 17:48 CET, Hugo Pereira Da Costa 		Details | Diff
GDB trace (3.95 KB, text/plain)
2012-02-05 18:27 CET, Thierry Vignaud 		Details
v3 (892 bytes, patch)
2012-02-05 18:53 CET, Hugo Pereira Da Costa 		Details | Diff
GDB trace (3.93 KB, text/plain)
2012-02-05 20:20 CET, Thierry Vignaud 		Details
Show Obsolete (6) View All Add an attachment (proposed patch, testcase, etc.)

Description Thierry Vignaud 2012-02-04 19:05:32 CET 
Description of problem:
metacity ( => gnome-classic, not gnome-shell & mutter) segfaults in oxygen theme.
If you look at the GDB trace, you see the theme engine tries to dereference a pointer in the NULL page

Version-Release number of selected component (if applicable):
lib64oxygen-gtk-1.2.0-2.mga2

How reproducible:
always

Steps to Reproduce:
1. switch Gtk+ theme to oxygen with gnome-tweak-tool
2. logout
3. login, session displays "an error happened"
4. run gdb from a console to see the trace
Thierry Vignaud 2012-02-04 19:05:58 CET

CC: (none) => hugo

Comment 1 Thierry Vignaud 2012-02-04 19:07:50 CET 
Created attachment 1492 [details]
GDB trace

As you see Oxygen::QtSettings::loadKdeIcons() calls (though std::string template) strlen on a pointer that is in the first page (the NULL page) and thus segfault
Thierry Vignaud 2012-02-04 19:08:08 CET

Severity: normal => major

Comment 2 Thierry Vignaud 2012-02-04 19:10:55 CET 
BTW you might want to look at https://bugs.mageia.org/buglist.cgi?cmdtype=runnamed&namedcmd=oxygen (also available from bugzilla>preferences>Saved Searches)
Comment 3 Hugo Pereira Da Costa 2012-02-05 15:22:52 CET 
The guilty code is (in details):

        gchar** gtkSearchPath;
        int nElements(0);
        gtk_icon_theme_get_search_path( gtk_icon_theme_get_default(), &gtkSearchPath, &nElements );
        for( int i=0; i<nElements; i++ ) { searchPath.insert( gtkSearchPath[i] ); }

So from the gtk method call, I was expecting, by construction, gtkSearchPath[i] not to be Null (never), or else it means nElements is just not calculated right. 
(or do I not read c++ right ?)

This to say: can be easily fixed in oxygen-gtk, but this rather looks like a Gtk bug to me. Maybe its worth reporting it there.
Comment 4 Hugo Pereira Da Costa 2012-02-05 15:24:38 CET 
Side note: can't reproduce. 
Could you also post the gtk version ?
Comment 5 Hugo Pereira Da Costa 2012-02-05 15:27:12 CET 
Created attachment 1493 [details]
patch (for testing)

Tentative patch.
Easy enough.
Could not test since I could not reproduce the crash in the first place.
Please report back and I'll push the patch upstream.
Comment 6 Hugo Pereira Da Costa 2012-02-05 15:27:42 CET 
(PS 2: please also report the metacity version. Mine might be too old)
Comment 7 Thierry Vignaud 2012-02-05 17:02:56 CET 
gtk+2.0-2.24.9-1.mga2
metacity-2.34.1-5.mga2
Comment 8 Hugo Pereira Da Costa 2012-02-05 17:22:24 CET 
thx !
I have gtk-2.24.8 and metacity-2.30.3-4.mga1
Will upgrade gtk (compiled from source)
Can't guaranty metacity upgrade.
Comment 9 Hugo Pereira Da Costa 2012-02-05 17:22:43 CET 
Also, I'm 32bits.
Comment 10 Thierry Vignaud 2012-02-05 17:40:10 CET 
Created attachment 1494 [details]
GDB trace with path applied

Now it segfaults on free()
Comment 11 Thierry Vignaud 2012-02-05 17:40:38 CET 
For the record, I'm running latest Mageia Cauldron.
Comment 12 Hugo Pereira Da Costa 2012-02-05 17:43:00 CET 
(I'm not :))
Anyway. I think the issue is that the initial output of gtk_icon_theme_get_search_path is 0.
hence the "free" issue. Will post another patch.
Comment 13 Hugo Pereira Da Costa 2012-02-05 17:48:01 CET 
Created attachment 1495 [details]
another patch

Another tentative patch. In replacement for (and not on top of) the previous patch that did not work.
Comment 14 Thierry Vignaud 2012-02-05 18:27:22 CET 
Created attachment 1496 [details]
GDB trace

still segfaulting...
Comment 15 Hugo Pereira Da Costa 2012-02-05 18:37:48 CET 
Well, 
since 
1/ I can't reproduce
2/ obviously gtkSearchPath is not null (from previous patch failing), but can't be freed (from previous crash report), it has to be a gtk bug (and/or meta city).
Nothing I can do about it :(
Comment 16 Hugo Pereira Da Costa 2012-02-05 18:42:31 CET 
PS: just updated to gtk+-2.24.9 and metacity-2.34.1
Comment 17 Thierry Vignaud 2012-02-05 18:45:44 CET 
Well only oxygen-gtk segfaults like this...
Default adwaita theme works just fine with metacity
Comment 18 Hugo Pereira Da Costa 2012-02-05 18:49:22 CET 
Well. that doesn't prove anything, sorry to say.
The other theme(s) likely don't call the guilty gtk function. Doesn't make it oxygen's fault, does it ? 

Anyway. after reading the documentation for gtk_icon_theme_get_search_path I'll give a shot to another patch (that uses g_strfreev() instead of g_free). Maybe that will fix the other crash.
Comment 19 Hugo Pereira Da Costa 2012-02-05 18:53:50 CET 
Created attachment 1497 [details]
v3
Comment 20 Thierry Vignaud 2012-02-05 20:20:28 CET 
Created attachment 1499 [details]
GDB trace

Still segfaulting...
Comment 21 Hugo Pereira Da Costa 2012-02-09 18:52:35 CET 
The last backtrace does not make much sense, sadly enough.
If the crash is in "if( gtkSearchPath[i] )"
it means that memory is corrupted. 
since the guy is direct out of a gtk call, it has to be a gtk bug.
nothing I can do about it.
Should rather be reported upstream.
Comment 22 Juan Luis Baptiste 2012-02-09 19:22:44 CET 
Done:

https://bugzilla.gnome.org/show_bug.cgi?id=669765
Comment 23 Juan Luis Baptiste 2012-02-10 04:53:35 CET 
The upstream bug was closed as not a GTK bug:


--- Comment #1 from Matthias Clasen <mclasen@redhat.com> 2012-02-10 01:42:56 UTC ---
Taking a quick look at some of the stacktraces in the Mandriva bug:

Gtk-CRITICAL **: IA__gtk_icon_theme_get_search_path: assertion
`GTK_IS_ICON_THEME (icon_theme)' failed

indicates that the GtkIconTheme object that gtk_icon_theme_get_search_path is
called on is not valid. Anything after that critical is not really relevant...
Olav Vitters 2012-02-10 07:58:18 CET

CC: (none) => olav

Comment 25 Hugo Pereira Da Costa 2012-02-10 13:39:26 CET 
oops wrong link.
Thierry Vignaud 2012-02-10 14:23:51 CET

Attachment 1493 is obsolete: 0 => 1

Thierry Vignaud 2012-02-10 14:23:56 CET

Attachment 1494 is obsolete: 0 => 1

Thierry Vignaud 2012-02-10 14:24:04 CET

Attachment 1495 is obsolete: 0 => 1

Thierry Vignaud 2012-02-10 14:24:08 CET

Attachment 1496 is obsolete: 0 => 1

Thierry Vignaud 2012-02-10 14:24:14 CET

Attachment 1497 is obsolete: 0 => 1

Thierry Vignaud 2012-02-10 14:24:22 CET

Attachment 1499 is obsolete: 0 => 1

Comment 27 Thierry Vignaud 2012-02-10 15:18:46 CET 
I'd to rediff the patch to apply cleanly on 1.2.0 (looks like you commited one of the test patch).
It definitively fix the bug one of my test machine.
I'll do further testing tonight on another one.

Interestingly, it doesn't happen in a chroot unless /var/run/dbus/ is binded on th chroot one.

Thanks for the patch!

Keywords: (none) => PATCH, UPSTREAM

Comment 28 Thierry Vignaud 2012-02-10 15:24:37 CET 
I'll put back the suggest tag on oxygen-gtk in gtk+2.0
Comment 29 Thierry Vignaud 2012-02-10 19:34:02 CET 
Checked on another test machine. Works fine too.
Thanks for the fix.
oxygen-gtk-1.2.0-3.mga2 is being uploaded with that fix.

Status: NEW => RESOLVED
Resolution: (none) => FIXED
Note You need to log in before you can comment on or make changes to this bug.