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 606068 - Setup libjpeg error handling earlier so random pointers in memory aren't called.
Setup libjpeg error handling earlier so random pointers in memory aren't called.
Status: RESOLVED FIXED
Product: gdk-pixbuf
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2010-01-05 00:39 UTC by Adam Langley
Modified: 2010-11-30 15:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix (1.73 KB, patch)
2010-01-05 01:27 UTC, Evan Martin (Chromium)
none Details | Review

Description Adam Langley 2010-01-05 00:39:46 UTC
The code in gdk-pixbuf/io-jpeg.c:gdk_pixbuf__jpeg_image_begin_load calls jpeg_create_decompress before this line:

context->cinfo.err = jpeg_std_error (&context->jerr.pub);

This means that if jpeg_create_decompress tries to error out it will treat random stack memory as a function pointer.

There's one case where this will start to happen a lot. If GTK+ is linked against libjpeg v7, but a binary pulls in v62 and libgtk then GTK+ will get the v62 functions. When it calls jpeg_create_* it will try to error out in jpeg_CreateDecompress and crash. This affects Chrome:

http://code.google.com/p/chromium/issues/detail?id=30288

Because of this, we (Chrome) are probably going to statically link libjpeg.
Comment 1 Evan Martin (Chromium) 2010-01-05 01:27:40 UTC
Created attachment 150810 [details] [review]
fix

Any ideas on how I can test I didn't break anything with this?
Comment 2 Matthias Clasen 2010-01-05 01:51:03 UTC
There is a bunch of tests for pixbuf error handling in gtk+/tests:

pixbuf-lowmem, pixbuf-random, pixbuf-randomly-modified
Comment 3 Matthias Clasen 2010-01-07 03:45:39 UTC
Seems to work fine here in cursory testing.
Comment 4 Evan Martin (Chromium) 2010-03-08 06:02:33 UTC
Ping.  Anything else I can do here to help?
Comment 5 Evan Martin (Chromium) 2010-03-08 06:03:27 UTC
Ah, I see it's been committed, thanks!

http://git.gnome.org/browse/gtk+/commit/?id=9fc436d810d12f75387bf8bfd440ef6db460500a
Comment 6 Evgeny 2010-05-02 06:45:03 UTC
I rebuild gtk+ with this patch, and Chromium still chashes.. Anybody knows, what's the trouble? (Gentoo, Gtk 2.18.9, Chromium 5.0.394.0)
Comment 7 Craig Schlenter 2010-11-19 15:55:26 UTC
The applied patch solves only part of the problem. Nothing has called sigsetjmp so when fatal_error_handler runs and does the siglongjmp it's jumping using a buffer that hasn't been set up and so it still explodes. It needs something like this:

( pasting it here 'cos it's probably whitespace damaged. Yell if I should generate a pristine copy and attach it please )

--- io-jpeg.c   2010-11-17 08:27:28.492046767 -0500
+++ io-jpeg.c.new       2010-11-19 07:39:10.908052266 -0500
@@ -663,6 +663,12 @@
        context->jerr.pub.output_message = output_message_handler;
        context->jerr.error = error;

+       if (sigsetjmp (context->jerr.setjmp_buffer, 1)) {
+               jpeg_destroy_decompress (&context->cinfo);
+               /* error should have been set by fatal_error_handler () */
+               return NULL;
+       }
+
       /* create libjpeg structures */
       jpeg_create_decompress (&context->cinfo);

I'm completely unfamiliar with libjpeg and the above is pretty much a copy and paste of what happens elsewhere so maybe there's something else that needs to be freed etc. so review sceptically please.

This seems to fix crbug.com/30288 on OpenSuse 11.3 btw. There's a backtrace of the crash without this patch in comment 84.

Comments?
Comment 8 Evan Martin (Chromium) 2010-11-22 22:43:06 UTC
Since the original bug is closed, it might be better to add your patch to a new bug.
Comment 9 Craig Schlenter 2010-11-30 15:55:26 UTC
I filed https://bugzilla.gnome.org/show_bug.cgi?id=636138 now.