Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler)
As someone who's been watching this bug for a while, I feel the need to interject to defend the Debian maintainers before they're alienated. Given how trivial a poppler-less xpdf package would be to create, the Debian maintainers probably have good reason not to package one. I'm guessing this is due to the fact that poppler/xpdf has had more than its fair share of security issues, and justifying a second copy of that code in Debian, and having to take responsibility for applying any security fixes to that code is too much work to justify for a single package, versus a library. As for the patch, yes it contains very few functional changes, but being too large by itself *is* enough to reject. The diff is extremely unlikely to apply cleanly to new versions of xpdf making it hard to port but even more problematically, it'll need to be updated each poppler release, and these tend to change the API a lot. What happens when functionality not present in the current version of poppler finally propagates in from xpdf, and configuration variables and methods are duplicated in the xpdf-specific global parameters class (e.g. enableT1lib)? xpdf and poppler will end up working with independent copies of the configuration variables and accessor methods. This is the fragile base class problem. This problem isn't an ABI issue, it's a software engineering dysfunction issue. Even forking xpdf and having that fork target poppler, as has been done/attempted before[1,2] is potentially problematic. In any normal development situation packages using libraries will only depend on functionality present in the libraries it uses at the time of release. Xpdf using poppler is the opposite. New releases of xpdf introduce functionality that the viewer depends on. Any version of xpdf using poppler will have to interact with versions of poppler that haven't yet incorporated this functionality. My point is that this problem is non-trivial to solve, and the Debian maintainers are already going above and beyond in producing a package that uses poppler. As far as I can see, the only people in a position to correctly maintain an xpdf version that works correctly with poppler is the poppler devs themselves. They care enough about the xpdf code base to fork it, perhaps they'll appreciate that trying to support packaging both xpdf and their code is hurting the former. Maybe this could be placed in a politely worded e-mail from a Debian maintainer. Lastly, despite the issues mentioned above I've also created a stripped down xpdf which doesn't replicate/extend the GlobalParams class. It stores xpdf-specific configuration variables in a new class, and only sets configuration variables from the parsed config file that can be accessed via the methods poppler's GlobalParams exposes [3]. This is primarily just so I can get a working version of xpdf on Ubunutu (and hence uses poppler 0.20) but I thought I might as well mention it. Francis [1] https://github.com/rbrito/xpdf-poppler [2] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=351279 [3] https://github.com/FrancisRussell/xpdf-lite -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler)
Michael Gilbert mgilb...@debian.org writes: #662882 https://bugs.launchpad.net/ubuntu/+source/xpdf/+bug/669211 (see comment 47) Those are this bug, and Ubuntu developers are responsible for their system preferring poppler's globalparams and pretty much breaking everything. They need to find their own solution, and they did for 12.10. I don't follow the Ubuntu packages closely, but due to the undefined nature of the bug, I'm 100% sure that the reports that this is _unfixed_ even in 12.10 https://bugs.launchpad.net/ubuntu/+source/xpdf/+bug/943195 are real. Saying there are potential security issues without evidence is blowing the problem out of proportion. If there is real evidence that there is a problem, I will certainly look at it, but guesses are not sufficient. So analysing and verifying that there is an _undefined behaviour_ type bug here is no such evidence for you? Also, the patch attached to this report is far too large. Any patch should address the known problems specifically, rather than just copying popper's globalparams. You didn't really look at the patch in much detail, did you? It is _not_ copying libpoppler's GlobalParams. It is _deriving from_ libpoppler's GlobalParams as defined in /usr/include/poppler/GlobalParams.h. Therefore, unless libpoppler's major version is bumped, it will only use its public ABI and hence continue to work even if libpoppler is upgraded. (As to Jens Stimpfle's suggestion of reverting to a poppler-less xpdf Version, yes that would be possible, but would lose all the Debian-specific work libpoppler has done. It depends on how much you value that. Personally, I am generally opposed to removing functionality which others have added.) Far too large: there are maybe 15-20 lines of functional changes in this patch, mainly changing function signatures for error propagation. The rest is _purely mechanical_. The C++ compiler and valgrind are your friends here. All I can say from 20+ years of C/C++ experience is that this patch is a definitive fix for this grave bug. I am severely disappointed by your handling of this issue, and if the quality of your comments doesn't improve noticeably -- sorry, I have nothing more to say. Regards, Wolfram. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler)
On Sun, Oct 07, 2012 at 03:30:08PM -0400, Michael Gilbert wrote: #622877 #640515 #606885 Not major i.e. release-critiical issues. I didn't claim /those/ where release-critical issues. Apart from that, broken printing (landscape), quiet aborts for password-protected PDF and apparently random segfaults are bugs which disqualify xpdf for production use, like computer pools. Those are this bug, and Ubuntu developers are responsible for their system preferring poppler's globalparams and pretty much breaking everything. They need to find their own solution, and they did for 12.10. I listed this example because the actual problem is explained there. Saying there are potential security issues without evidence is blowing the problem out of proportion. If there is real evidence that there is a problem, I will certainly look at it, but guesses are not sufficient. It /is/ a problem to have a package which builds (by chance) an invalid binary (passing wrong struct to library functions, luckily a bigger one!). I and many others consider this definitely a security problem. There may be no security problem with the current Debian release as I write this (I don't know), but one may appear at any time, for example with any poppler upgrade, even for unchanged xpdf, and the new poppler package being perfectly valid. After passing a wrong struct to a function, the behaviour is undefined. Additionally, when the struct is not all zeroes (it isn't), the real-world chance of real-world nasty behaviour goes significantly up and malicious abuse is generally real-world possible. Also, the patch attached to this report is far too large. Agreed. And it really fix the code-duplication problem. Any patch should address the known problems specifically Disagreed. There is /no/ fixing of specific bugs related to the GlobalParams problem. They all exist only because the Debian xpdf package links poppler (and breaks the build). Which can't be done correctly without a /huge/ amount of work, because poppler has moved too far from its origins. Here's my patch: Change the xpdf package so it doesn't try to link poppler anymore. Please do us the favor. The lenny version worked just fine. And really, it's a threat to keep an invalid build. I also wouldn't mind helping getting a poppler-less xpdf package in good shape for wheezy. -- Have a nice day Jens Stimpfle -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler)
control: severity -1 important On Thu, Oct 4, 2012 at 8:37 PM, Jens Stimpfle wrote: To reiterate again, the current wheezy xpdf package is in a terribly defunct state, possibly imposing severe security problems and should under no circumstances be included into the stable release. No it isn't. Bugs remain, but that is the case for every package in every release. Here are other related bugs: #622877 #640515 #606885 Not major i.e. release-critiical issues. #628591 That's been done for a long long time. It's an incompatibility with an old expat. #662882 https://bugs.launchpad.net/ubuntu/+source/xpdf/+bug/669211 (see comment 47) Those are this bug, and Ubuntu developers are responsible for their system preferring poppler's globalparams and pretty much breaking everything. They need to find their own solution, and they did for 12.10. Saying there are potential security issues without evidence is blowing the problem out of proportion. If there is real evidence that there is a problem, I will certainly look at it, but guesses are not sufficient. Also, the patch attached to this report is far too large. Any patch should address the known problems specifically, rather than just copying popper's globalparams. In the future, please do not override maintainer severities without a good reason. No one likes bts ping pong. Best wishes, Mike -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler)
severity 658264 grave thanks Fri, 10 Aug 2012 18:38:38 +0200, David Madore david+b...@madore.org: Is there any hope of this fix entering Debian Wheezy before it completely freezes? To reiterate what has been said earlier in the thread, accessing uninitialized memory has potential security issues, so this should be treated as more than a simple bugfix. We haven't heard of Michael for some time, and the xpdf package hasn't notably changed in the last 6 months, so we could assume the maintainer has no interest in getting the package to a sane state? To reiterate again, the current wheezy xpdf package is in a terribly defunct state, possibly imposing severe security problems and should under no circumstances be included into the stable release. Here are other related bugs: #622877 #640515 #606885 #628591 #662882 https://bugs.launchpad.net/ubuntu/+source/xpdf/+bug/669211 (see comment 47) Having read all that I am at a loss understanding how those bugs' severities could be lowered to not being RC. So, sorry for being so impolite to have bug control requests included (dunno if it works, at all). At my workplace, we have rebuilt the lenny xpdf package for wheezy. And we think, for Debian, too, it would be better to revert to the old version (upstream is close to dead anyway). Poppler might be out of reach. -- Have a nice day Jens Stimpfle -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler)
Just to confirm that Wolfram's patch works beautifully for me, and fixes the utter brokenness of xpdf on Ubuntu Precise as well as its not-so-obvious brokenness on Debian Wheezy. So, many thanks to him for figuring this one out and fixing it! Is there any hope of this fix entering Debian Wheezy before it completely freezes? To reiterate what has been said earlier in the thread, accessing uninitialized memory has potential security issues, so this should be treated as more than a simple bugfix. (Also, if Debian fixes this and Ubuntu copies the fix, a huge number of users will thank you: see Ubuntu bug 943195.) In the meantime, if it's of use to anyone, I built some packages of xpdf (3.03-10.dmadore) with this fix (as well as that of bug #671765 which is needed to compile on Ubuntu): they are in URL: ftp://quatramaran.ens.fr/pub/madore/misc/xpdf-bug658264/ (compiled versions for i386 and amd64 were done on Debian Wheezy). -- David A. Madore ( http://www.madore.org/~david/ ) -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler)
Just out of interest: Is this really a namespace conflict? As I understand the code, xpdf and libpoppler should want to use an object of same class from the same namespace, but due to some reason, the class code was duplicated to xpdf. I'm not c++ expert, but perhaps this was to make linking of xpdf simpler. I did a short analysis before reading this bug report by myself ( http://www.halfdog.net/Security/2012/XpdfCrashAnalysisUbuntuPrecise/ ). So from my opinion, with the patch attached, this will fix the problem only for some combination of xpdf/poppler versions and will make problem reappear in future. Could someone confirm that? -- http://www.halfdog.net/ PGP: 156A AE98 B91F 0114 FE88 2BD8 C459 9386 feed a bee -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: closed by Michael Gilbert michael.s.gilb...@gmail.com (Re: Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler))
And here is the corresponding patch for stable/squeeze.
Luckily it is a bit simpler.
Regards,
Wolfram.
description: fix type clash GlobalParam xpdf vs. poppler
author: Wolfram Gloger bugzil...@malloc.de
debian-bug: http://bugs.debian.org/658264
Index: xpdf-3.02/xpdf/GlobalParams.cc
===
--- xpdf-3.02.orig/xpdf/GlobalParams.cc 2012-02-09 14:25:22.0 +0100
+++ xpdf-3.02/xpdf/GlobalParams.cc 2012-02-09 17:20:35.0 +0100
@@ -48,19 +48,11 @@
#endif
#if MULTITHREADED
-# define lockGlobalParamsgLockMutex(mutex)
-# define lockUnicodeMapCache gLockMutex(unicodeMapCacheMutex)
-# define lockCMapCache gLockMutex(cMapCacheMutex)
-# define unlockGlobalParams gUnlockMutex(mutex)
-# define unlockUnicodeMapCache gUnlockMutex(unicodeMapCacheMutex)
-# define unlockCMapCache gUnlockMutex(cMapCacheMutex)
+# define lockGlobalParamsgLockMutex(xpdf_mutex)
+# define unlockGlobalParams gUnlockMutex(xpdf_mutex)
#else
# define lockGlobalParams
-# define lockUnicodeMapCache
-# define lockCMapCache
# define unlockGlobalParams
-# define unlockUnicodeMapCache
-# define unlockCMapCache
#endif
#include NameToUnicodeTable.h
@@ -601,20 +593,26 @@
// parsing
//
-GlobalParams::GlobalParams(char *cfgFileName) {
- UnicodeMap *map;
+GlobalParams::GlobalParams(char *cfgFileName)
+#if POPPLER_GLOBAL
+: poppler_GlobalParams()
+#endif
+{
GString *fileName;
FILE *f;
- int i;
+
+#if POPPLER_GLOBAL
+#undef globalParams
+ globalParams = this;
+#endif
#if MULTITHREADED
- gInitMutex(mutex);
- gInitMutex(unicodeMapCacheMutex);
- gInitMutex(cMapCacheMutex);
+ gInitMutex(xpdf_mutex);
#endif
initBuiltinFontTables();
+#if !POPPLER_GLOBAL
// scan the encoding in reverse because we want the lowest-numbered
// index for each char name ('space' is encoded twice)
macRomanReverseMap = new NameToCharCode();
@@ -637,6 +635,7 @@
unicodeMaps = new GHash(gTrue);
cMapDirs = new GHash(gTrue);
toUnicodeDirs = new GList();
+#endif
displayFonts = new GHash();
displayCIDFonts = new GHash();
displayNamedCIDFonts = new GHash();
@@ -662,11 +661,6 @@
psImageableURX = psPaperWidth;
psImageableURY = psPaperHeight;
psCrop = gTrue;
- psExpandSmaller = gFalse;
- psShrinkLarger = gTrue;
- psCenter = gTrue;
- psDuplex = gFalse;
- psLevel = psLevel2;
psFile = NULL;
psFonts = new GHash();
psNamedFonts16 = new GList();
@@ -675,6 +669,7 @@
psEmbedTrueType = gTrue;
psEmbedCIDPostScript = gTrue;
psEmbedCIDTrueType = gTrue;
+#if !POPPLER_GLOBAL
psPreload = gFalse;
psOPI = gFalse;
psASCIIHex = gFalse;
@@ -688,10 +683,12 @@
#endif
textPageBreaks = gTrue;
textKeepTinyChars = gFalse;
+#endif
fontDirs = new GList();
initialZoom = new GString(125);
continuousView = gFalse;
enableT1lib = gTrue;
+#if !POPPLER_GLOBAL
enableFreeType = gTrue;
antialias = gTrue;
vectorAntialias = gTrue;
@@ -702,8 +699,10 @@
screenGamma = 1.0;
screenBlackThreshold = 0.0;
screenWhiteThreshold = 1.0;
+#endif
urlCommand = NULL;
movieCommand = NULL;
+#if !POPPLER_GLOBAL
mapNumericCharNames = gTrue;
mapUnknownCharNames = gFalse;
createDefaultKeyBindings();
@@ -747,6 +746,9 @@
residentUnicodeMaps-add(map-getEncodingName(), map);
map = new UnicodeMap(UCS-2, gTrue, mapUCS2);
residentUnicodeMaps-add(map-getEncodingName(), map);
+#else
+ createDefaultKeyBindings();
+#endif
// look for a user config file, then a system-wide config file
f = NULL;
@@ -928,6 +930,9 @@
GString *cmd, *incFile;
char *p1, *p2;
FILE *f2;
+ GBool pBool;
+ intpInt;
+ double pDouble;
// break the line into tokens
tokens = new GList();
@@ -1009,12 +1014,15 @@
} else if (!cmd-cmp(psCrop)) {
parseYesNo(psCrop, psCrop, tokens, fileName, line);
} else if (!cmd-cmp(psExpandSmaller)) {
- parseYesNo(psExpandSmaller, psExpandSmaller,
- tokens, fileName, line);
+ if (parseYesNo(psExpandSmaller, pBool,
+ tokens, fileName, line))
+ setPSExpandSmaller(pBool);
} else if (!cmd-cmp(psShrinkLarger)) {
- parseYesNo(psShrinkLarger, psShrinkLarger, tokens, fileName, line);
+ if (parseYesNo(psShrinkLarger, pBool, tokens, fileName, line))
+ setPSShrinkLarger(pBool);
} else if (!cmd-cmp(psCenter)) {
- parseYesNo(psCenter, psCenter, tokens, fileName, line);
+ if (parseYesNo(psCenter, pBool, tokens, fileName, line))
+ setPSCenter(pBool);
} else if (!cmd-cmp(psDuplex)) {
parseYesNo(psDuplex, psDuplex, tokens, fileName, line);
} else if (!cmd-cmp(psLevel)) {
@@ -1031,21 +1039,25 @@
parseYesNo(psEmbedCIDTrueType, psEmbedCIDTrueType,
tokens, fileName, line);
} else if (!cmd-cmp(psPreload)) {
- parseYesNo(psPreload, psPreload, tokens, fileName, line);
Bug#658264: closed by Michael Gilbert michael.s.gilb...@gmail.com (Re: Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler))
tag 658264 patch
thanks
Michael Gilbert michael.s.gilb...@gmail.com writes:
Since you're interested in
this, it would be great if you could spend some time trying to find a
solution.
Ok, here is a patch, to be applied after all other debian patches. It
is not nice, but IMHO should work. valgrind doesn't complain any more.
An issue remains with the format string of the error() messages but that
is a different, less serious bug.
Regards,
Wolfram.
===File fix-globalparams-clash.patch===
description: fix type clash GlobalParam xpdf vs. poppler
author: Wolfram Gloger bugzil...@malloc.de
debian-bug: http://bugs.debian.org/658264
Index: xpdf-3.03/xpdf/GlobalParams.cc
===
--- xpdf-3.03.orig/xpdf/GlobalParams.cc 2012-02-08 16:42:24.0 +0100
+++ xpdf-3.03/xpdf/GlobalParams.cc 2012-02-08 21:26:51.0 +0100
@@ -48,19 +48,11 @@
#endif
#if MULTITHREADED
-# define lockGlobalParamsgLockMutex(mutex)
-# define lockUnicodeMapCache gLockMutex(unicodeMapCacheMutex)
-# define lockCMapCache gLockMutex(cMapCacheMutex)
-# define unlockGlobalParams gUnlockMutex(mutex)
-# define unlockUnicodeMapCache gUnlockMutex(unicodeMapCacheMutex)
-# define unlockCMapCache gUnlockMutex(cMapCacheMutex)
+# define lockGlobalParamsgLockMutex(xpdf_mutex)
+# define unlockGlobalParams gUnlockMutex(xpdf_mutex)
#else
# define lockGlobalParams
-# define lockUnicodeMapCache
-# define lockCMapCache
# define unlockGlobalParams
-# define unlockUnicodeMapCache
-# define unlockCMapCache
#endif
#include NameToUnicodeTable.h
@@ -626,20 +618,26 @@
// parsing
//
-GlobalParams::GlobalParams(char *cfgFileName) {
- UnicodeMap *map;
+GlobalParams::GlobalParams(char *cfgFileName)
+#if POPPLER_GLOBAL
+: poppler_GlobalParams()
+#endif
+{
GString *fileName;
FILE *f;
- int i;
+
+#if POPPLER_GLOBAL
+#undef globalParams
+ globalParams = this;
+#endif
#if MULTITHREADED
- gInitMutex(mutex);
- gInitMutex(unicodeMapCacheMutex);
- gInitMutex(cMapCacheMutex);
+ gInitMutex(xpdf_mutex);
#endif
initBuiltinFontTables();
+#if !POPPLER_GLOBAL
// scan the encoding in reverse because we want the lowest-numbered
// index for each char name ('space' is encoded twice)
macRomanReverseMap = new NameToCharCode();
@@ -662,6 +660,7 @@
unicodeMaps = new GHash(gTrue);
cMapDirs = new GHash(gTrue);
toUnicodeDirs = new GList();
+#endif
fontFiles = new GHash(gTrue);
fontDirs = new GList();
ccFontFiles = new GHash(gTrue);
@@ -688,11 +687,6 @@
psImageableURX = psPaperWidth;
psImageableURY = psPaperHeight;
psCrop = gTrue;
- psExpandSmaller = gFalse;
- psShrinkLarger = gTrue;
- psCenter = gTrue;
- psDuplex = gFalse;
- psLevel = psLevel2;
psFile = NULL;
psResidentFonts = new GHash(gTrue);
psResidentFonts16 = new GList();
@@ -702,44 +696,27 @@
psEmbedCIDPostScript = gTrue;
psEmbedCIDTrueType = gTrue;
psFontPassthrough = gFalse;
- psPreload = gFalse;
- psOPI = gFalse;
- psASCIIHex = gFalse;
+
psUncompressPreloadedImages = gFalse;
psRasterResolution = 300;
psRasterMono = gFalse;
psAlwaysRasterize = gFalse;
- textEncoding = new GString(Latin1);
-#if defined(WIN32)
- textEOL = eolDOS;
-#elif defined(MACOS)
- textEOL = eolMac;
-#else
- textEOL = eolUnix;
-#endif
- textPageBreaks = gTrue;
- textKeepTinyChars = gFalse;
+
initialZoom = new GString(125);
continuousView = gFalse;
enableT1lib = gTrue;
- enableFreeType = gTrue;
+
disableFreeTypeHinting = gFalse;
- antialias = gTrue;
- vectorAntialias = gTrue;
+
antialiasPrinting = gFalse;
- strokeAdjust = gTrue;
- screenType = screenUnset;
- screenSize = -1;
- screenDotRadius = -1;
- screenGamma = 1.0;
- screenBlackThreshold = 0.0;
- screenWhiteThreshold = 1.0;
+
minLineWidth = 0.0;
drawAnnotations = gTrue;
overprintPreview = gFalse;
launchCommand = NULL;
urlCommand = NULL;
movieCommand = NULL;
+#if !POPPLER_GLOBAL
mapNumericCharNames = gTrue;
mapUnknownCharNames = gFalse;
createDefaultKeyBindings();
@@ -779,6 +756,9 @@
residentUnicodeMaps-add(map-getEncodingName(), map);
map = new UnicodeMap(UCS-2, gTrue, mapUCS2);
residentUnicodeMaps-add(map-getEncodingName(), map);
+#else
+ createDefaultKeyBindings();
+#endif
// look for a user config file, then a system-wide config file
f = NULL;
@@ -958,6 +938,9 @@
GString *cmd, *incFile;
char *p1, *p2;
FILE *f2;
+ GBool pBool;
+ intpInt;
+ double pDouble;
// break the line into tokens
tokens = new GList();
@@ -1023,12 +1006,15 @@
} else if (!cmd-cmp(psCrop)) {
parseYesNo(psCrop, psCrop, tokens, fileName, line);
} else if (!cmd-cmp(psExpandSmaller)) {
- parseYesNo(psExpandSmaller, psExpandSmaller,
-tokens, fileName, line);
+
Bug#658264: closed by Michael Gilbert michael.s.gilb...@gmail.com (Re: Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler))
reopen 658264 severity 658264 important tag 658264 help retitle 658264 possible memory corruption in GlobalParams thanks and xpdf.xx:159 is exactly the forementioned problematic: globalParams = new GlobalParams(cfgFileName); I have now invested \approx 6h in this report and am 100% sure it is a grave bug. Please consider re-opening. OK, reopening. I'm probably not going to get around to looking at this for a while, so I'm tagging help. Since you're interested in this, it would be great if you could spend some time trying to find a solution. Thanks, Mike -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: closed by Michael Gilbert michael.s.gilb...@gmail.com (Re: Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler))
ow...@bugs.debian.org (Debian Bug Tracking System) writes: Make sure you've also updated to the expa from squeeze. This bug has been reported many times now: #628591, #603153, etc. Upgrading libexpat1 indeed changes behaviour and avoids the obvious bug symptoms, presumably due to a differently aligned heap/stack. That explains why only a few people have experienced this. However, this changes _absolutely nothing_ with regard to the grave bug in Debian's xpdf or -- if you will -- poppler. Like I have stated, due to the incompatibility of GlobalParams in xpdf and poppler, xpdf is accessing uninitilized memory, leading to undefined behaviour. In the best case, it 'works' by chance. But on the next upgrade, whether it be libexpat or libc or whatever, the bug _surely_ will bite again! At worst, this will become a security issue sometime. If you don't want to reproduce my instrumentation, please try valgrind xpdf any.pdf: ==5746== Conditional jump or move depends on uninitialised value(s) ==5746==at 0x5289CD4: Gfx::go(bool) (in /usr/lib/libpoppler.so.13.0.0) ==5746==by 0x528A104: Gfx::display(Object*, bool) (in /usr/lib/libpoppler.so.13.0.0) ==5746==by 0x52D3B35: Page::displaySlice(OutputDev*, double, double, int, bool, bool, int, int, int, int, bool, Catalog*, bool (*)(void*), void*, bool (*)(Annot*, void*), void*) (in /usr/lib/libpoppler.so.13.0.0) ==5746==by 0x41C87C: PDFCore::needTile(PDFCorePage*, int, int) (PDFCore.cc:890) ==5746==by 0x41BC39: PDFCore::update(int, int, int, double, int, bool, bool, bool) (PDFCore.cc:712) ==5746==by 0x4248E2: XPDFCore::update(int, int, int, double, int, bool, bool, bool) (XPDFCore.cc:288) ==5746==by 0x419E98: PDFCore::displayPage(int, double, int, bool, bool) (PDFCore.cc:301) ==5746==by 0x42E3FC: XPDFViewer::displayPage(int, double, int, bool, bool) (XPDFViewer.cc:463) ==5746==by 0x42D881: XPDFViewer::XPDFViewer(XPDFApp*, GooString*, int, GooString*, bool, GooString*, GooString*) (XPDFViewer.cc:302) ==5746==by 0x4225CD: XPDFApp::open(GooString*, int, GooString*, GooString*) (XPDFApp.cc:228) ==5746==by 0x42B405: main (xpdf.cc:313) ==5746== Uninitialised value was created by a heap allocation ==5746==at 0x4C24DFA: operator new(unsigned long) (vg_replace_malloc.c:261) ==5746==by 0x42AB77: main (xpdf.cc:159) and xpdf.xx:159 is exactly the forementioned problematic: globalParams = new GlobalParams(cfgFileName); I have now invested \approx 6h in this report and am 100% sure it is a grave bug. Please consider re-opening. Thanks, Wolfram Gloger -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#658264: xpdf totally unusable due to memory corruption in globalParams class (namespace conflict with libpoppler)
Package: xpdf
Version: 3.02-12+squeeze1
Severity: grave
After upgrading to squeeze, xpdf crashes on startup, even without any
input file:
% /usr/bin/xpdf any.pdf
xpdf: pthread_mutex_lock.c:62: __pthread_mutex_lock: Assertion
`mutex-__data.__owner == 0' failed.
Abgebrochen (core dumped)
I also compiled poppler_0.16.7-2 and xpdf_3.03-8 from unstable
and the problem persists -- there is no crash but an indefinite
hang.
Source inspection and -instrumentation clearly show the explanation,
however I have no patch or even a good suggestion.
There is a class 'GlobalParams' in poppler and in xpdf.
Its ABI is defined in
/usr/include/poppler/GlobalParams.h (A)
and
xpdf-version/xpdf/GlobalParams.h(B)
and these 2 files have diverged considerably.
In xpdf, the class is actually instantiated by
globalParams = new GlobalParams(cfgFileName);
in xpdf-version/xpdf/xpdf.cc and this uses (A). However, all the
GlobalParams-functions in libpoppler used by xpdf call the ABI (B).
This results in memory corruption, notably of the 'mutex' member.
If you are not convinced, apply the following to GlobalParams.h
in _both_ source packages:
--- poppler-0.16.7/poppler/GlobalParams.cc~ 2010-12-27 21:44:28.0
+0100
+++ poppler-0.16.7/poppler/GlobalParams.cc 2012-02-01 16:26:24.0
+0100
@@ -74,7 +74,7 @@
#endif
#if MULTITHREADED
-# define lockGlobalParamsgLockMutex(mutex)
+# define lockGlobalParamsdo { fprintf(stderr, t=%p
m=%p\n,this,mutex);gLockMutex(mutex); }while(0)
# define lockUnicodeMapCache gLockMutex(unicodeMapCacheMutex)
# define lockCMapCache gLockMutex(cMapCacheMutex)
# define unlockGlobalParams gUnlockMutex(mutex)
(also applies to the xpdf version)
When running xpdf such instrumented, I get:
% LD_LIBRARY_PATH=/home/wg/Debian/poppler-0.16.7/poppler/.libs \
~/Debian/xpdf-3.03/build/xpdf.real any.pdf
t=0x18ae010 m=0x18ae188
t=0x18ae010 m=0x18ae188
t=0x18ae010 m=0x18ae188
...
t=0x18ae010 m=0x18ae0f0
This proves that the same mutex object is accessed at two locations.
As said, I have no easy fix. I'd say including the GlobalParams class
and also the globalParams global variable in libpoppler was a horrible
design decision. But going back to a poppler-independent xpdf package
now also doesn't look good. :-(
The problem was already clearly pointed out 8 months ago:
https://bugs.launchpad.net/ubuntu/+source/xpdf/+bug/669211
esp. comment 47..
Regards,
Wolfram.
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
