| Summary: | Crash when calling cmsGetColorSpace | ||
|---|---|---|---|
| Product: | poppler | Reporter: | Marek Kasik <mkasik> |
| Component: | general | Assignee: | poppler-bugs <poppler-bugs> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | CC: | junrrein, mitch, monsta |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
reproducer
Don't crash when calling cmsGetColorSpace() Don't crash when calling cmsGetColorSpace() |
||
|
Description
Marek Kasik
2016-09-20 09:04:41 UTC
Created attachment 126645 [details] [review] Don't crash when calling cmsGetColorSpace() This patch fixes the problem for me. (In reply to Marek Kasik from comment #1) > Created attachment 126645 [details] [review] [review] > Don't crash when calling cmsGetColorSpace() > > This patch fixes the problem for me. Is that the only place where all those global variables could be used uninitialized? GfxColorSpace::setupColorProfiles() is already protected to ensure it only happens once even if called multiple times, so I think it would be simpler to call it unconditionally before trying to use RGBProfile. But I would make this generic, the current use of all those global vars is very weak in my opinion. I would make a global singleton to contain those variables or something similar. Created attachment 126698 [details] [review] Don't crash when calling cmsGetColorSpace() (In reply to Carlos Garcia Campos from comment #2) > (In reply to Marek Kasik from comment #1) > > Created attachment 126645 [details] [review] [review] [review] > > Don't crash when calling cmsGetColorSpace() > > > > This patch fixes the problem for me. > > Is that the only place where all those global variables could be used > uninitialized? GfxColorSpace::setupColorProfiles() is already protected to > ensure it only happens once even if called multiple times, so I think it > would be simpler to call it unconditionally before trying to use RGBProfile. Some of those variables can be used uninitialized via getters like: GfxColorSpace::getRGBProfile() GfxColorSpace::getDisplayProfile() The XYZ2DisplayTransform is also used in some ::parse() methods but these usages are guarded by consequent NULL checks. I've changed the patch to call the setup of color profiles unconditionally. Thanks Marek, I confirm this fixes the issue with atril-thumbnailer crashing (Atril is a fork of Evince). I've patched poppler 0.41.0 in Ubuntu 16.04 and the crash is now gone. For reference - Ubuntu bug report about that: https://bugs.launchpad.net/bugs/1635812 *** Bug 98678 has been marked as a duplicate of this bug. *** Is theree a poppler release which contains the fix? The bug isn't closed, so not, it hasn't even been commited to the git repo. Personally i'm not convinced by the patch but since Carlos took lead on the review i'm leaving it to him to decide (In reply to Albert Astals Cid from comment #7) > The bug isn't closed, so not, it hasn't even been commited to the git repo. > > Personally i'm not convinced by the patch but since Carlos took lead on the > review i'm leaving it to him to decide What are your concerns? (In reply to Carlos Garcia Campos from comment #8) > (In reply to Albert Astals Cid from comment #7) > > The bug isn't closed, so not, it hasn't even been commited to the git repo. > > > > Personally i'm not convinced by the patch but since Carlos took lead on the > > review i'm leaving it to him to decide > > What are your concerns? I feel the solution is a bit too much of "using a cannon to kill flies" from the architectural point of view (since from the performance point of view probably will be fine) Any news here? This is a blocker for GIMP because poppler simply crashes without any way for us to prevent that. We'd like to depend on a poppler version that has this fixed :) |
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.