GNOME Bugzilla – Bug 604004
SGI: allocate memory consistently
Last modified: 2009-12-09 16:26:33 UTC
Created attachment 149276 [details] [review] Proposed fix: avoid double frees Freeing both rows[0] and rows caused glibc to flag invalid pointers (though I'm not sure this is a real security issue). The attached patch makes the SGI plugin handle this the same way as pixel/pixels with rows/rows_data (which appeases glibc).
Sorry, I think that patch is nonsense. Can you elaborate plase?
You're right in that this change didn't fix the double free that glibc detected(*), it seems the patch attached to bug #604002 does it (though I don't understand why). Still, the patch would make allocation of the rows buffer similar to how pixel/pixels is handled, making the code more consistent: As it is now, both pixels and rows are arrays the values of which point to their respective starts in a large buffer. With pixel, this buffer is allocated as "pixels" and pixels[0..n] are filled in a subsequent loop, but with rows it is allocated as "rows[0]" and rows[1..n] are filled afterwards. It wouldn't make the code more correct, but easier understandable because two essentially same actions would be done the same way. (*): This one (current master, a736b8f972a): --- 8< --- This is a development version of GIMP. Debug messages may appear here. sgiGetRow(sgip, rows[i], 127, 0) failed! [...] sgiGetRow(sgip, rows[i], 100, 0) failed! *** glibc detected *** /home/nils/gimp-2.7/lib/gimp/2.0/plug-ins/file-sgi: free(): invalid next size (fast): 0x0000000000cc3570 *** ======= Backtrace: ========= /lib64/libc.so.6[0x3840874576] /home/nils/gimp-2.7/lib/gimp/2.0/plug-ins/file-sgi[0x40230a] /home/nils/gimp-2.7/lib/libgimp-2.0.so.0(gimp_main+0x698)[0x7f66d81baa88] /lib64/libc.so.6(__libc_start_main+0xfd)[0x384081eb1d] /home/nils/gimp-2.7/lib/gimp/2.0/plug-ins/file-sgi[0x401a89] ======= Memory map: ======== [...] 34aa866000-34aa868000 rw-p 00000000 00:00 0 plug-in 'file-sgi' aborted before sending its procedure return values --- >8 ---
Created attachment 149321 [details] [review] Proposed patch: allocate memory more consistently In contrast to the first patch (attachment #149276 [details]), this one makes allocation for pixels and rows data more consistent, i.e. it keeps pointers into the buffer in the pixels/rows arrays and allocates the buffers as the respective first element. Let me know which one you like more.
I think it doesn't really matter, since both patches make the code more consistent, however (since we talk about readability), I would say "guchar *pixels[]" instead of using a double pointer.
I don't care enough either. I'd say go for it and commit it.
Review of attachment 149321 [details] [review]: committed: b7ae59e SGI: allocate memory more consistently