From: SourceForge.net <no...@so...> - 2009-01-31 16:01:25
|
Bugs item #2494093, was opened at 2009-01-08 17:02 Message generated for change (Comment added) made by matzek You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=2494093&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 10. Objects Group: obsolete: 8.5.5 Status: Open Resolution: None Priority: 6 Private: No Submitted By: Matthias Kraft (matzek) Assigned to: Don Porter (dgp) Summary: buffer overflow in AppendUnicodeToUnicode() -> SEGFAULT Initial Comment: Hi *, initially I got a core dump on AIX 5.2 in FlushChannel() of tclIO.c using Tcl 8.4.19. I tracked it down to the memcpy() call in AppendUnicodeToUnicode() in tclStringObj.c that is getting a wrong destination pointer in case the attemptckrealloc() call (doubling available space for StringPtr) failed. Once I understood how this all comes to happen, the following simple script provokes this crash even on Linux ... proc bar {} { set f [string repeat "\xDE\xAD\xBE\xEF" 1024] ;# for the crash in AppendUnicodeToUnicodeRep() #set f [string repeat "DEADBEEF" 1024] ;# for a crash in AppendUtfToUtfRep() that I have not investigated further set r "" set i 0 while {1} { puts -nonewline stdout "$i\r" set fl [string length $f] if {($fl != [string length $r]) || 1} { append r [string range $f 0 $fl] } incr i $fl } } bar I don't quite understand why it gets the wrong address there, yet. Note: It may happen that the crash occurs at different positions, or leaves unreadable core dumps behind. This could be avoided with setting the environment variable MALLOC_CHECK_=2 on a fairly recent Linux system, or MALLOCTYPE=debug on AIX. The crash also depends on how much free memory is available. If you need more information, core dumps, stack traces ... just ask. One furher thing, I polluted the sources with printf()s, the crashing call to AppendUnicodeToUnicodeRep() generated: +++ alloced tmpString (0x2ab8abe8) of size uallocated (67126290) +++ copying 8192 bytes from 0x20b59ff8 to 0x32b8cbf8 1. +++ -> fprintf( stderr, "+++ alloced tmpString (0x%lx) of size uallocated (%ld)\n", tmpString, STRING_SIZE(stringPtr->uallocated) ); just before stringPtr = tmpString; 2. +++ -> fprintf( stderr, "+++ copying %ld bytes from 0x%lx to 0x%lx\n", appendNumChars * sizeof(Tcl_UniChar), unicode, (VOID *)(stringPtr->unicode + stringPtr->numChars)); just before the memcpy() call. -- Matthias Kraft ---------------------------------------------------------------------- >Comment By: Matthias Kraft (matzek) Date: 2009-01-31 17:01 Message: Update: The patch works on Linux x86_64 with top of core-8-5-branch. Variables can now again hold (1 << 30) - appendNumChars bytes before panic(). But: The patch as it is now, does not compile on AIX using xlC: .../generic/tclStringObj.c", line 355.17: 1506-226 (S) The ":" operator is not allowed between "int" and "char*". that is from an attempt to compile a patched 8.4.19. I will look into this, but maybe not today as I am working from home and the VPN is sluggish and slow today -> no fun... -- Matthias Kraft ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-28 20:04 Message: Sorry Don, I was off to a workshop and have to look after other paid work now. I hope to find time for testing tomorrow or Friday... kind regards -- Matthias Kraft ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-27 17:58 Message: Agreed. Matthias???? ---------------------------------------------------------------------- Comment By: Andreas Kupries (andreas_kupries) Date: 2009-01-27 01:15 Message: I do not believe so. We are not stressing the memory subsystem with extra-large allocs. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-27 00:27 Message: no reply from matzek. aku, is your testing enough to confirm this is ok for backport? ---------------------------------------------------------------------- Comment By: Andreas Kupries (andreas_kupries) Date: 2009-01-22 23:00 Message: Fix: The NULL after the Tcl_Panic in STRING_NOMEM needs a cast to (char*). ---------------------------------------------------------------------- Comment By: Andreas Kupries (andreas_kupries) Date: 2009-01-22 22:44 Message: Don, please review your changes. On HPUX IA64 I am now getting errors and these are in the macros you modified. When I diffed to history I saw that they became quite complex. andreask@bigsur03:~/workbench/z> tail log/tcl-8.6/make.log Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2720 # Illegal types found in conditional operator: 'long' and 'char *'. copyStringPtr = stringAlloc(STRING_UALLOC(0)); ^^^^^^^^^^^ Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2723 # Illegal types found in conditional operator: 'long' and 'char *'. copyStringPtr = stringAlloc(srcStringPtr->uallocated); ^^^^^^^^^^^ Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2789 # Illegal types found in conditional operator: 'long' and 'char *'. stringPtr = stringAlloc(STRING_UALLOC(0)); ^^^^^^^^^^^ make: *** [tclStringObj.o] Error 2 andreask@bigsur03:~/workbench/z> ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-21 22:31 Message: fix committed to HEAD. Please test and after you confirm success, I will backport. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-21 21:29 Message: ok, I think fixing the panic on attempt problem corrects both issues. When the attemp panicked prematurely, that cut off the "slow growth" portion of the string growth algorithm, which would in fact reduce the allocatable buffer by about a factor two. Patch on the way... ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-21 18:49 Message: matzek, can you provide your evidence that the commit has dropped the max value size in half? Thanks for pointing out the problem with attempt. The new panic is in the wrong place. Trickier than I thought. ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-15 10:03 Message: >From a mail I sent to the tcl-core list: I want to give two points to your attention that concern me with the fix of this bug. But first thank you very much for helping to find the problem and then fixing it promptly! 1) The fix as now implemented restricts the content of variables to INT_MAX/4 which is 512MB! To my mind this is comparatively small and should be documented somewhere. Please notice that this is also half the size, of what was possible in 8.4.19 / 8.5.6. 2) In tclStringObj.c is a lengthy documentation of the memory allocation algorithm for strings. This is now obsolete in the case when more than INT_MAX/4 memory is free and could be allocated. As in this case we panic() before even coming to the attemptckalloc(). What you should make with this information? Well, I don't know. I just wanted to make sure it doesn't get lost and someone has to learn it the hard way. I can live with the fix now. Where is the use case that needs so much memory? Consider vfs::memchan. This is where I stumbled over the issue. I thought I use vfs::memchan to be able to transparently uncompress and evaluate the crc of archives... ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-09 16:35 Message: trouble in STRING_SIZE() also fixed in all branches. ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-09 11:45 Message: ok, looked into 8.5 again: it now seems to be some kind of integer overflow, or something in this direction. My Linux build is 64 bit, that means sizeof(size_t) is 8, while sizeof(int) is 4. The core dump now shows: - the memcpy() call in AppendUnicodeToUnicodeep() crashes again: #0 0x00007febad876dd3 in memcpy () from /lib64/libc.so.6 #1 0x00007febae0a82d1 in AppendUnicodeToUnicodeRep (objPtr=0x625990, unicode=0x63bafc, appendNumChars=4096) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1336 - the other values are: (gdb) print *stringPtr $2 = {numChars = 1073737728, allocated = 16384, uallocated = 4294967296, hasUnicode = 1, unicode = {222, 173}} (gdb) print numChars $3 = 1073741824 - $3 == 1<<30 == 1GB - (gdb) print numChars - stringPtr->numChars $6 = 4096 - stringPtr->uallocated == 1<<32 == 4GB which means the attemptckrealloc() has not failed so far my observations, maybe I have time to come back to it later today ... ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-09 10:30 Message: Testing the patch with my customized Tcl 8.4.19 on AIX, shows that it seems to be fixed there ... that means I am running into out-of-memory now and up to that point the buffers seem to be ok ... ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-09 09:46 Message: Sorry Don, but it is not fixed. I tested core-8.5-branch with your changes this morning and it still dumps core on Linux at the same place with the same stack trace ... ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-08 18:59 Message: fixed in all branches ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-08 18:49 Message: Found it. Problem is in the STRING_UALLOC() macro: #define STRING_UALLOC(numChars) \ (numChars * sizeof(Tcl_UniChar)) It needs changing to: #define STRING_UALLOC(numChars) \ ((numChars) * sizeof(Tcl_UniChar)) ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-08 18:26 Message: sorry, I think my hexadigit counting was off by one. The crash happens reallocating for 2**28 characters. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-08 17:49 Message: In case it's not already obvious, the crash happens when "numChars" value escapes the range of a signed int. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-01-08 17:32 Message: if this is fixed reasonably quickly, the fix will also be part of 8.4.20. ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-08 17:28 Message: This bug is present in Tcl 8.5.6, but I have not the freedom of choice and am still stuck at 8.4.19. So if someone can fix it for 8.5 I kindly ask to provide a patch that could be applied against 8.4, too, or back ported. ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2009-01-08 17:19 Message: Do you mean you have confidence that the bug is no longer present after 8.4 ? What are your reasons to stick to 8.4 ? ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-08 17:14 Message: As I now have missed the bus anyway, there is time to ask for a patch that could be ported back to 8.4 :-) ... ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-08 17:09 Message: Stack trace on Linux x86_64 with Tcl 8.5.6: Program terminated with signal 11, Segmentation fault. #0 0x00007f5443b0ddd3 in memcpy () from /lib64/libc.so.6 (gdb) bt #0 0x00007f5443b0ddd3 in memcpy () from /lib64/libc.so.6 #1 0x00007f544433f23e in AppendUnicodeToUnicodeRep (objPtr=0x624330, unicode=0x63a55c, appendNumChars=4096) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1336 #2 0x00007f544433efb3 in Tcl_AppendObjToObj (objPtr=0x624330, appendObjPtr=0x6244e0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1230 #3 0x00007f54443536e1 in TclPtrSetVar (interp=0x605790, varPtr=0x6067b0, arrayPtr=0x0, part1Ptr=0x0, part2Ptr=0x0, newValuePtr=0x6244e0, flags=516, index=1) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclVar.c:1881 #4 0x00007f54442deed5 in TclExecuteByteCode (interp=0x605790, codePtr=0x6367f0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclExecute.c:2952 #5 0x00007f5444337011 in TclObjInterpProcCore (interp=0x605790, procNameObj=0x623b20, skip=1, errorProc=0x7f544433771f <MakeProcError>) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclProc.c:1748 #6 0x00007f5444336f0b in TclObjInterpProc (clientData=0x6272f0, interp=0x605790, objc=1, objv=0x6065c0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclProc.c:1642 #7 0x00007f5444279c39 in TclEvalObjvInternal (interp=0x605790, objc=1, objv=0x6065c0, command=0x634b46 "bar\n", length=4, flags=0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:3690 #8 0x00007f544427af60 in TclEvalEx (interp=0x605790, script=0x6347d0 "set i 1234567890\nset n /dev/zero\n\nproc foo {n i} {\n set f [open $n r]\n fconfigure $f -translation binary -buffering full -buffersize 4096\n\n upvar #0 data($n,raw) r\n set r \"\"\n\n while {$i"..., numBytes=890, flags=0, line=46) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:4338 #9 0x00007f544427a4b5 in Tcl_EvalEx (interp=0x605790, script=0x6347d0 "set i 1234567890\nset n /dev/zero\n\nproc foo {n i} {\n set f [open $n r]\n fconfigure $f -translation binary -buffering full -buffersize 4096\n\n upvar #0 data($n,raw) r\n set r \"\"\n\n while {$i"..., numBytes=890, flags=0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:4043 #10 0x00007f544430e3e1 in Tcl_FSEvalFileEx (interp=0x605790, pathPtr=0x624690, encodingName=0x0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclIOUtil.c:1814 #11 0x00007f5444316ed0 in Tcl_Main (argc=-1, argv=0x7fff4c7ba4a8, appInitProc=0x400925 <Tcl_AppInit>) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclMain.c:441 #12 0x000000000040091e in main (argc=2, argv=0x7fff4c7ba498) at /home/makr/cvs/tccvs/tcl-8.5/unix/../unix/tclAppInit.c:87 ---------------------------------------------------------------------- Comment By: Matthias Kraft (matzek) Date: 2009-01-08 17:06 Message: Stack trace on AIX with Tcl 8.4.19: Segmentation fault in . at 0xf050 0x0000f050 warning: Unable to access address 0xf050 from core (dbx) where warning: Unable to access address 0xf050 from core warning: Unable to access address 0xf050 from core warning: Unable to access address 0xf04c from core warning: Unable to access address 0xf04c from core warning: Unable to access address 0xf050 from core warning: Unable to access address 0xf050 from core warning: Unable to access address 0xf04c from core warning: Unable to access address 0xf04c from core warning: Unable to access address 0xf050 from core .() at 0xf050 AppendUnicodeToUnicodeRep(objPtr = 0x20996ca0, unicode = 0x20b59ff8, appendNumChars = 4096), line 1260 in "tclStringObj.c" Tcl_AppendObjToObj(objPtr = 0x20996ca0, appendObjPtr = 0x209967c0), line 1154 in "tclStringObj.c" TclPtrSetVar(interp = 0x20090e08, varPtr = 0x2ff21718, arrayPtr = (nil), part1 = warning: Unable to access address 0x20b24ff8 from core (invalid char ptr (0x20b24ff8)), part2 = (nil), newValuePtr = 0x209967c0, flags = 516), line 1666 in "tclVar.c" TclExecuteByteCode(interp = 0x20090e08, codePtr = 0x20b46eb0), line 2072 in "tclExecute.c" TclCompEvalObj(interp = 0x20090e08, objPtr = 0x20996958), line 1107 in "tclExecute.c" unnamed block $b1240, line 1180 in "tclProc.c" TclObjInterpProc(clientData = 0x20aa4fe0, interp = 0x20090e08, objc = 1, objv = 0x2ff21ca8), line 1180 in "tclProc.c" TclEvalObjvInternal(interp = 0x20090e08, objc = 1, objv = 0x2ff21ca8, command = warning: Unable to access address 0x20a73ff4 from core (invalid char ptr (0x20a73ff4)), length = 4, flags = 0), line 3219 in "tclBasic.c" Tcl_EvalEx(interp = 0x20090e08, script = warning: Unable to access address 0x20a73d00 from core (invalid char ptr (0x20a73d00)), numBytes = 760, flags = 0), line 4011 in "tclBasic.c" Tcl_FSEvalFile(interp = 0x20090e08, pathPtr = 0x204b5f70), line 1776 in "tclIOUtil.c" Tcl_Main(argc = -1, argv = 0x2ff222c0, appInitProc = 0x2001b92c), line 295 in "tclMain.c" main(argc = 2, argv = 0x2ff222b8), line 288 in "instsh.c" ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=2494093&group_id=10894 |