| Summary: | graphics/freeimage - rework, upgrade, unbreak | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Mikhail Teterin <mi> | ||||||||||||
| Component: | Individual Port(s) | Assignee: | Mikhail Teterin <mi> | ||||||||||||
| Status: | Closed Overcome By Events | ||||||||||||||
| Severity: | Affects Some People | CC: | amdmi3, danfe, freebsd-2024, jbeich, jrm, kevlo, monwarez, ndowens04, oliver, sawp, thierry, tj | ||||||||||||
| Priority: | --- | Keywords: | patch | ||||||||||||
| Version: | Latest | ||||||||||||||
| Hardware: | Any | ||||||||||||||
| OS: | Any | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Mikhail Teterin
2017-04-15 20:53:03 UTC
Comment on attachment 181815 [details] Update, ubreak freeimage Doesn't build in poudriere: FreeImage/J2KHelper.cpp:24:10: fatal error: 'openjpeg.h' file not found #include <openjpeg.h> ^ http://sprunge.us/TLcK (build log) imv starts crashing with this update when trying to open some JPEGs.
I think only if they have Exiv data. See [1] for an example image.
Other formats are ok AFAICT.
#0 FreeImage_CloneTag (tag=0x80b088110) at Metadata/FreeImageTag.cpp:96
#1 0x0000000800c317e7 in FreeImage_CloneMetadata (dst=0x80b088070, src=0x80b088000)
at FreeImage/BitmapAccess.cpp:1305
#2 0x0000000800bbb0e9 in FreeImage_ConvertTo32Bits (dib=0x80b088000)
at FreeImage/Conversion32.cpp:213
#3 0x00000000004072d7 in bg_new_img (data=0x7fffffffdc60) at src/loader.c:264
#4 0x00000008012cac35 in thread_start (curthread=0x8061ac400)
at /usr/src/lib/libthr/thread/thr_create.c:289
#5 0x0000000000000000 in ?? ()
[1] https://pkg.tobik.me/patches/example.jpg
> FreeImage/J2KHelper.cpp:24:10: fatal error: 'openjpeg.h' file not found Uh, sorry, pkgconfig is required for building -- just add "pkgconfig" into the "USES" line -- I don't want to post a whole new diff just for this :-) > imv starts crashing with this update when trying to open some JPEGs. Investigating... BTW, imv itself would not build for me here: cc -c -O2 -pipe -march=core2 -fstack-protector -fno-strict-aliasing -std=c99 -I/opt/include/SDL2 -I/opt/include/directfb -D_REENTRANT -D_REENTRANT -D_THREAD_SAFE -D_XOPEN_SOURCE=700 -DIMV_VERSION=\""v2.1.3"\" -o build/loader.o src/loader.c 1 error generated. In file included from src/viewport.c:18: src/viewport.h:21:10: fatal error: 'SDL2/SDL.h' file not found #include <SDL2/SDL.h> The file exists: -rw-r--r-- 1 root wheel 4101 Mar 15 14:07 /opt/include/SDL2/SDL.h but ${LOCALBASE}/include is not in compiler's include-path for some reason... Created attachment 181828 [details] Fix crash inside FreeImage_CloneTag (In reply to Tobias Kortkamp from comment #2) Ok, the crash -- with a debuggable imv -- is thus: #0 0x0000000800b65558 in FreeImage_CloneTag (tag=0x8124a1168) at Metadata/FreeImageTag.cpp:96 src_tag = 0x0 dst_tag = 0x8124bfd00 clone = 0x8124a13a8 message = 0x7fffdfbfb410 "" #1 0x0000000800c21a51 in FreeImage_CloneMetadata (dst=0x8124a10c8, src=0x8124a1058) at FreeImage/BitmapAccess.cpp:1305 [...] The crash is due to an attempt to dereference a NULL-pointer (src_tag), and is easily prevented (see attached diff), but what I do not understand is that the line where it happens is inside a try-block -- should not the segfault be caught by that and NULL returned by the function as a result? Freeimage library itself is compiled with -fexceptions -- is there anything else necessary for try/catch to work? Thoughts? Created attachment 181829 [details]
Update, ubreak freeimage
This version addresses the problems uncovered so far and includes some more patches found on the upstream's SourceForge.
Remark: you should define the licenses. It comes with 3 license files: - license-fi.txt - license-gplv2.txt - license-gplv3.txt And also `make regression-test' core dumps: $ make regression-test cd /usr/ports/graphics/freeimage/work/FreeImage/TestAPI && ./testAPI FreeImage version: 3.17.0 This program uses FreeImage, a free, open source image library supporting all common bitmap formats. See http://freeimage.sourceforge.net for details bitmap type 35 (JPEG-XR): JPEG XR image format (jxr,wdp,hdp) bitmap type 34 (WebP): Google WebP image format (webp) bitmap type 33 (RAW): RAW camera image (3fr,arw,bay,bmq,cap,cine,cr2,crw,cs1,dc2,dcr,drf,dsc,dng,erf,fff,ia,iiq,k25,kc2,kdc,mdc,mef,mos,mrw,nef,nrw,orf,pef,ptx,pxn,qtk,raf,raw,rdc,rw2,rwl,rwz,sr2,srf,srw,sti,x3f) bitmap type 32 (PICT): Macintosh PICT (pct,pict,pic) bitmap type 31 (PFM): Portable floatmap (pfm) bitmap type 30 (JP2): JPEG-2000 File Format (jp2) bitmap type 29 (J2K): JPEG-2000 codestream (j2k,j2c) bitmap type 28 (EXR): ILM OpenEXR (exr) bitmap type 27 (SGI): SGI Image Format (sgi,rgb,rgba,bw) bitmap type 26 (HDR): High Dynamic Range Image (hdr) bitmap type 25 (GIF): Graphics Interchange Format (gif) bitmap type 24 (DDS): DirectX Surface (dds) bitmap type 23 (XPM): X11 Pixmap Format (xpm) bitmap type 22 (XBM): X11 Bitmap Format (xbm) bitmap type 21 (CUT): Dr. Halo (cut) bitmap type 20 (PSD): Adobe Photoshop (psd) bitmap type 19 (WBMP): Wireless Bitmap (wap,wbmp,wbm) bitmap type 18 (TIFF): Tagged Image File Format (tif,tiff) bitmap type 17 (TARGA): Truevision Targa (tga,targa) bitmap type 16 (RAS): Sun Raster Image (ras) bitmap type 15 (PPMRAW): Portable Pixelmap (RAW) (ppm) bitmap type 14 (PPM): Portable Pixelmap (ASCII) (ppm) bitmap type 13 (PNG): Portable Network Graphics (png) bitmap type 12 (PGMRAW): Portable Greymap (RAW) (pgm) bitmap type 11 (PGM): Portable Greymap (ASCII) (pgm) bitmap type 10 (PCX): Zsoft Paintbrush (pcx) bitmap type 9 (PCD): Kodak PhotoCD (pcd) bitmap type 8 (PBMRAW): Portable Bitmap (RAW) (pbm) bitmap type 7 (PBM): Portable Bitmap (ASCII) (pbm) bitmap type 6 (MNG): Multiple-image Network Graphics (mng) bitmap type 5 (IFF): IFF Interleaved Bitmap (iff,lbm) bitmap type 4 (KOALA): C64 Koala Graphics (koa) bitmap type 3 (JNG): JPEG Network Graphics (jng) bitmap type 2 (JPEG): JPEG - JFIF Compliant (jpg,jif,jpeg,jpe) bitmap type 1 (ICO): Windows Icon (ico) bitmap type 0 (BMP): Windows or OS/2 Bitmap (bmp) testAllocateCloneUnload ... *** Signal 11 Stop. make: stopped in /usr/ports/graphics/freeimage (In reply to Thierry Thomas from comment #6) > It comes with 3 license files: > - license-fi.txt > - license-gplv2.txt > - license-gplv3.txt Khmm, so, what should the LICENSE-line be? > `make regression-test' core dumps I haven't seen this -- not amd64, not on i386... Which platform were you using? Could you build WITH_DEBUG=yes and try to obtain the stack? Also, are you sure, you've been using the latest version of the upgrade -- one with additional patches? Thanks! Created attachment 182171 [details]
Defining the licenses
About the licenses, I would define them like in the proposed patch.
But beware! I'm not a specialist, please check.
About the tests: for me it's reproducible on 11.0-STABLE amd64: testAllocateCloneUnload ... Program received signal SIGSEGV, Segmentation fault. 0x0000000800876510 in FreeImage_CloneTag (tag=0x804c3e138) at Metadata/FreeImageTag.cpp:96 96 dst_tag->id = src_tag->id; (gdb) bt #0 0x0000000800876510 in FreeImage_CloneTag (tag=0x804c3e138) at Metadata/FreeImageTag.cpp:96 #1 0x000000080092d40a in FreeImage_Clone (dib=0x804c3e000) at FreeImage/BitmapAccess.cpp:605 #2 0x0000000000403fd4 in testClone (lpszPathName=0x4096a0 "exif.jpg") at testImageType.cpp:37 #3 0x00000000004040cc in testAllocateCloneUnload (lpszPathName=0x4096a0 "exif.jpg") at testImageType.cpp:56 #4 0x0000000000402cc0 in main (argc=1, argv=0x7fffffffe1b8) at MainTestSuite.cpp:65 (In reply to Thierry Thomas from comment #10) > About the tests: for me it's reproducible on 11.0-STABLE amd64 I don't see it on 10.3/amd64, but it would seem, you do NOT have the files/patch-FreeImageTag.cpp in your port's directory, which means, you aren't using the latest version of my update. The src_tag should be checked against NULL... "make test" fails on head@317660 amd64 as well:
testAllocateCloneUnload ...
Process 81907 stopped
* thread #1, stop reason = signal SIGSEGV: invalid address (fault address: 0xffffffffbf00000f)
frame #0: libfreeimage.so.3`::FreeImage_CloneTag(tag=0x0000000804e3e138) at FreeImageTag.cpp:96
93 FITAGHEADER *dst_tag = (FITAGHEADER *)clone->data;
94
95 // tag ID
-> 96 dst_tag->id = src_tag->id;
97 // tag key
98 if(src_tag->key) {
99 dst_tag->key = (char*)malloc((strlen(src_tag->key) + 1) * sizeof(char));
(lldb) bt
* thread #1, stop reason = signal SIGSEGV: invalid address (fault address: 0xffffffffbf00000f)
* frame #0: libfreeimage.so.3`::FreeImage_CloneTag(tag=0x0000000804e3e138) at FreeImageTag.cpp:96
frame #1: libfreeimage.so.3`::FreeImage_Clone(dib=0x0000000804e3e000) at BitmapAccess.cpp:605
frame #2: testAPI`testClone(lpszPathName="exif.jpg") at testImageType.cpp:37
frame #3: testAPI`testAllocateCloneUnload(lpszPathName="exif.jpg") at testImageType.cpp:56
frame #4: testAPI`main(argc=1, argv=0x00007fffffffe200) at MainTestSuite.cpp:65
frame #5: testAPI`_start(ap=<unavailable>, cleanup=<unavailable>) at crt1.c:72
(lldb) fr v
(FITAG *) tag = 0x0000000804e3e138
(FITAG *) clone = 0x0000000804e3e4b0
(const char *) message = 0x0000000804e1be20 "!"
(FITAGHEADER *) src_tag = 0xffffffffbeffffff
(FITAGHEADER *) dst_tag = 0x0000000804e2dfe0
(lldb) p *src_tag
(FITAGHEADER) $2 = (key = <no value available>, description = <no value available>, id = 0, type = 0, count = 0, length = 0, value = 0x0000000000000000)
(lldb) l 82
82 FITAG * DLL_CALLCONV
83 FreeImage_CloneTag(FITAG *tag) {
84 if (!tag || !tag->data) return NULL;
85
86 // allocate a new tag
87 FITAG *clone = FreeImage_CreateTag();
88 if(!clone) return NULL;
89
90 try {
91 // copy the tag
which maybe related to the following
testAllocateCloneUnload ...
=================================================================
==32979==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000fd8 at pc 0x000800af636d bp 0x7fffffffb4b0 sp 0x7fffffffb4a8
WRITE of size 8 at 0x602000000fd8 thread T0
#0 0x800af636c in processExifTag(FIBITMAP*, FITAG*, char*, int, TagLib::MDMODEL) Source/Metadata/Exif.cpp:473:14
#1 0x800aefbd6 in jpeg_read_exif_dir(FIBITMAP*, unsigned char const*, unsigned int, unsigned int, unsigned int, int, TagLib::MDMODEL) Source/Metadata/Exif.cpp:704:5
#2 0x800aebcbf in jpeg_read_exif_profile Source/Metadata/Exif.cpp:870:10
#3 0x800c74cbd in read_markers(jpeg_decompress_struct*, FIBITMAP*) Source/FreeImage/PluginJPEG.cpp:733:5
#4 0x800c6e30a in Load(FreeImageIO*, void*, int, int, void*) Source/FreeImage/PluginJPEG.cpp:1281:4
#5 0x800d0a7c6 in FreeImage_LoadFromHandle Source/FreeImage/Plugin.cpp:388:24
#6 0x800d0a978 in FreeImage_Load Source/FreeImage/Plugin.cpp:408:22
#7 0x4a1c43 in testClone(char const*) TestAPI/testImageType.cpp:34:10
#8 0x4a1edb in testAllocateCloneUnload(char const*) TestAPI/testImageType.cpp:56:17
#9 0x49fddf in main TestAPI/MainTestSuite.cpp:65:2
#10 0x41047f in _start /usr/src/lib/csu/amd64/crt1.c:72:7
0x602000000fd8 is located 0 bytes to the right of 8-byte region [0x602000000fd0,0x602000000fd8)
allocated by thread T0 here:
#0 0x48882c in __interceptor_malloc (TestAPI/testAPI+0x48882c)
#1 0x800af5cdf in processExifTag(FIBITMAP*, FITAG*, char*, int, TagLib::MDMODEL) Source/Metadata/Exif.cpp:408:28
#2 0x800aefbd6 in jpeg_read_exif_dir(FIBITMAP*, unsigned char const*, unsigned int, unsigned int, unsigned int, int, TagLib::MDMODEL) Source/Metadata/Exif.cpp:704:5
#3 0x800aebcbf in jpeg_read_exif_profile Source/Metadata/Exif.cpp:870:10
#4 0x800c74cbd in read_markers(jpeg_decompress_struct*, FIBITMAP*) Source/FreeImage/PluginJPEG.cpp:733:5
#5 0x800c6e30a in Load(FreeImageIO*, void*, int, int, void*) Source/FreeImage/PluginJPEG.cpp:1281:4
#6 0x800d0a7c6 in FreeImage_LoadFromHandle Source/FreeImage/Plugin.cpp:388:24
#7 0x800d0a978 in FreeImage_Load Source/FreeImage/Plugin.cpp:408:22
#8 0x4a1c43 in testClone(char const*) TestAPI/testImageType.cpp:34:10
#9 0x4a1edb in testAllocateCloneUnload(char const*) TestAPI/testImageType.cpp:56:17
#10 0x49fddf in main TestAPI/MainTestSuite.cpp:65:2
#11 0x41047f in _start /usr/src/lib/csu/amd64/crt1.c:72:7
#12 0x8006f3fff (<unknown module>)
SUMMARY: AddressSanitizer: heap-buffer-overflow Source/Metadata/Exif.cpp:473:14 in processExifTag(FIBITMAP*, FITAG*, char*, int, TagLib::MDMODEL)
Shadow bytes around the buggy address:
0x4c04000001a0: fa fa 00 fa fa fa 00 00 fa fa 02 fa fa fa fd fa
0x4c04000001b0: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fd
0x4c04000001c0: fa fa 00 fa fa fa 00 04 fa fa 00 05 fa fa 04 fa
0x4c04000001d0: fa fa fd fa fa fa 00 fa fa fa fd fa fa fa 00 fa
0x4c04000001e0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa 00 fa
=>0x4c04000001f0: fa fa 04 fa fa fa 00 fa fa fa 00[fa]fa fa fa fa
0x4c0400000200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x4c0400000210: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x4c0400000220: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x4c0400000230: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x4c0400000240: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==32979==ABORTING
*** Error code 1
ASan may work fine on 10.3-RELEASE e.g., $ pkg install llvm40 $ export CFLAGS=-fsanitize=address LDFLAGS=-fsanitize=address $ export CC=clang40 CXX=clang++40 CPP=clang-cpp40 $ make clean test Created attachment 182262 [details]
Update, ubreak freeimage, declare LICENSE
Thank you very much for the testing. Running the test-executable under valgrind I was able to identify -- and fix -- a number of different problems.
Please, give this new iteration a spin at your convenience.
Regarding the licensing, the FreeImage code itself comes under the "FIPL" license (Free Image Public License). The two versions of GPL found in the tarball are for the various third-party libraries bundled therein. Since the port will not be using the bundled code -- indeed, it will not even extract it -- the license can simply be FIPL itself.
Thanks!
"make test" succeeds on head@317660 amd64 but ASan is still not happy. freeimage-3.16.0_2 (current) is unaffected.
testMemIO ...
=================================================================
==91210==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffcb21 at pc 0x000000475e7e bp 0x7fffffffc600 sp 0x7fffffffbdb0
READ of size 290 at 0x7fffffffcb21 thread T0
#0 0x475e7d in __asan_memcpy (TestAPI/testAPI+0x475e7d)
#1 0x800d1649b in _MemoryWriteProc(void*, unsigned int, unsigned int, void*) Source/FreeImage/FreeImageIO.cpp:118:2
#2 0x800c914ac in FreeImage_WriteMemory Source/FreeImage/MemoryIO.cpp:228:11
#3 0x800af3318 in tiff_write_ifd(FIBITMAP*, FREE_IMAGE_MDMODEL, FIMEMORY*) Source/Metadata/Exif.cpp:1133:4
#4 0x800af1674 in tiff_get_ifd_profile Source/Metadata/Exif.cpp:1218:18
#5 0x800c68e88 in WriteMetadata(tagPKImageEncode*, FIBITMAP*) Source/FreeImage/PluginJXR.cpp:766:7
#6 0x800c617d5 in Save(FreeImageIO*, FIBITMAP*, void*, int, int, void*) Source/FreeImage/PluginJXR.cpp:1406:3
#7 0x800d0b2ee in FreeImage_SaveToHandle Source/FreeImage/Plugin.cpp:455:19
#8 0x800c906ed in FreeImage_SaveToMemory Source/FreeImage/MemoryIO.cpp:103:11
#9 0x4aaa64 in testSaveMemIO(char const*) TestAPI/testMemIO.cpp:36:2
#10 0x4ab1fb in testMemIO(char const*) TestAPI/testMemIO.cpp:144:2
#11 0x49fe13 in main TestAPI/MainTestSuite.cpp:75:2
#12 0x41047f in _start /usr/src/lib/csu/amd64/crt1.c:72:7
Address 0x7fffffffcb21 is located in stack of thread T0 at offset 673 in frame
#0 0x800af1b8f in tiff_write_ifd(FIBITMAP*, FREE_IMAGE_MDMODEL, FIMEMORY*) Source/Metadata/Exif.cpp:1048
This frame has 26 object(s):
[32, 40) '__first.i'
[64, 72) '__last.i'
[96, 97) '__comp.i'
[112, 120) 'retval.i.i138'
[144, 152) 'retval.i141'
[176, 184) 'retval.i.i'
[208, 216) 'retval.i'
[240, 264) '__annotator.i'
[304, 312) 'retval.i29.i.i'
[336, 344) 'retval.i21.i.i'
[368, 376) 'retval.i.i.i'
[400, 408) '__t1.addr.i.i.i.i'
[432, 440) '__t1.addr.i.i.i'
[464, 472) 'coerce.i.i'
[496, 504) 'coerce4.i.i'
[528, 536) 'coerce8.i.i'
[560, 568) 'tag'
[592, 616) 'vTagList'
[656, 660) 'ifd_offset'
[672, 673) 'empty_byte'
[688, 696) 'agg.tmp' <== Memory access at offset 673 partially underflows this variable
[720, 728) 'agg.tmp29' <== Memory access at offset 673 partially underflows this variable
[752, 754) 'nde' <== Memory access at offset 673 partially underflows this variable
[768, 770) 'tag_id54' <== Memory access at offset 673 partially underflows this variable
[784, 786) 'tag_type' <== Memory access at offset 673 partially underflows this variable
[800, 804) 'tag_count' <== Memory access at offset 673 partially underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (TestAPI/testAPI+0x475e7d) in __asan_memcpy
Shadow bytes around the buggy address:
0x4ffffffff910: f1 f1 f1 f1 00 f2 f2 f2 00 f2 f2 f2 01 f2 00 f2
0x4ffffffff920: f2 f2 00 f2 f2 f2 00 f2 f2 f2 00 f2 f2 f2 00 00
0x4ffffffff930: 00 f2 f2 f2 f2 f2 00 f2 f2 f2 00 f2 f2 f2 00 f2
0x4ffffffff940: f2 f2 00 f2 f2 f2 00 f2 f2 f2 00 f2 f2 f2 00 f2
0x4ffffffff950: f2 f2 00 f2 f2 f2 00 f2 f2 f2 00 00 00 f2 f2 f2
=>0x4ffffffff960: f2 f2 04 f2[01]f2 00 f2 f2 f2 00 f2 f2 f2 02 f2
0x4ffffffff970: 02 f2 02 f2 04 f3 f3 f3 00 00 00 00 00 00 00 00
0x4ffffffff980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x4ffffffff990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x4ffffffff9a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x4ffffffff9b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==91210==ABORTING
I "test" the last patch with stuntrally(I guess the later use it, in someways), and it didn't crash, and also look to work faster(maybe there is another reason) Are there remaining issues to be resolved, or is this just waiting for someone to commit? - poudriere testport 10/11 i386/amd64 looks good - `make test` reports lots of "TIFFFieldWithTag: Internal error, unknown tag 0xXXXX". - portlint -C complains about patches not being generated with `make makepatch`, but otherwise looks good. (In reply to Joseph Mingrone from comment #17) I keep meaning to investigate the problem reported in comment #15. But the current version of the patch is already a vast improvement over the status quo, so, maybe, it is worth committing as is... (In reply to Mikhail T. from comment #18) This bug should not exist now. I have since upgraded freeimage and builds Closing per comment #19 and lack of additional feedback since. |