Summary: | Valgrind shows use of uninitialised memory in clist code | ||
---|---|---|---|
Product: | Ghostscript | Reporter: | Robin Watts <robin.watts> |
Component: | Graphics Library | Assignee: | Ray Johnston <ray.johnston> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P4 | ||
Version: | master | ||
Hardware: | PC | ||
OS: | Linux | ||
Customer: | Word Size: | --- | |
Attachments: |
valgrind.txt
patch.txt patch2.txt |
Description
Robin Watts
2010-07-21 18:28:31 UTC
Created attachment 6523 [details]
test.pdf
The test PDF file.
I am regenerating the valgrind log now and will attach it in a moment. The errors fall into 2 categories. 1) All but the last error in the file appear to be to do with encoding a bitmap from a cached character where the buffer appears to have uninitialised data in it. I have a workaround that stops these warnings: base/gdevprn.c: line 122 onwards for ( space = space_params->BufferSpace; ; ) { base = (reallocate ? (byte *)gs_resize_object(buffer_memory, *the_memory, space, "cmd list buffer") : gs_alloc_bytes(buffer_memory, space, "cmd list buffer")); if (base != 0) + { + if (!reallocate) + memset(base, 0, space); /* RJW */ + break; + } if (bufferSpace_is_exact || (space >>= 1) < PRN_MIN_BUFFER_SPACE) break; } Clearly this is not the correct solution (for no other reason but that it leaves the (extra) memory uncleared in the reallocate case), but it suffices to stop the warnings in this case. 2) The second case is to do with strange results within an ftell call. I have a similar workaround for that: gxclist.c: line 728 int /* ret 0 all-ok, -ve error code, or +1 ok w/low-mem warning */ clist_end_page(gx_device_clist_writer * cldev) { int code = cmd_write_buffer(cldev, cmd_opv_end_page); cmd_block cb; int ecode = 0; + memset(&cb, 0, sizeof(cb)); /* RJW */ if (code >= 0) { /* * Write the terminating entry in the block file. * Note that because of copypage, there may be many such entries. */ It remains to be seen if these solve the original customers problem, but even if they do, clearly we'd like a better solution. Created attachment 6524 [details]
valgrind.txt
Valgrind log that shows the problems
The line numbers given are within the gs 8.71 source, but the same warnings are given in HEAD. Created attachment 6782 [details]
patch.txt
Better fix for Part 1 of the bug; in the fapi code when we copy from unaligned to aligned bitmaps, set the padding pixels to 0.
Part 1 fixed in revision 11759. Created attachment 6784 [details]
patch2.txt
Removing patch.txt as it was the wrong one.
Also attaching patch2.txt, the correct (IMHO) fix for part 2 of the bug.
In clist_end_page we write out a cmd_block, and fail to initialise it all. Even if we *did* initialise all the members, it would still (potentially) have uninitialised data in due to the structure padding on some architectures. As such memsetting the entire structure to zero before starting to work on it seems correct to me. I am clusterpushing this now, and will commit after conferring with Ray.
Fix committed in revision 11764. |