Bug 127639
| Summary: | Segfault in x_realloc devel/ccache | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Mel <mel.xyzzy> | ||||
| Component: | Individual Port(s) | Assignee: | Michael Johnson <ahze> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | CC: | ahze | ||||
| Priority: | Normal | ||||||
| Version: | Latest | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
Responsible Changed From-To: freebsd-ports-bugs->ahze Over to maintainer (via the GNATS Auto Assign Tool) Hello all, I found and fixed a rather interesting bug in ccache=E2=80=99s x_realloc() function, which tries to read out too many bytes from the _old_ storage after allocating the new storage. The obvious fix is to make it use realloc(3) instead. Interestingly enough, there is an =E2=80=9Cif (ptr)=E2=80=9D in the (origin= al) code which is always true, since the =E2=80=9Cif (!ptr)=E2=80=9D case was alread= y handled=E2=80=A6 seems as if the author of that code did not know what he was doing. Effects of the bug: =E2=80=A2 confirmed: segfault abortion (DoS) =E2=80=A2 don=E2=80=99t know if it can lead to data corruption, don=E2=80= =99t think so Fix: http://www.mirbsd.org/cvs.cgi/ports/devel/ccache/patches/patch-util_c bye, //mirabilos, who first thought his memcpy(3) was broken=E2=80=A6 --=20 Sometimes they [people] care too much: pretty printers [and syntax highligh= - ting, d.A.] mechanically produce pretty output that accentuates irrelevant detail in the program, which is as sensible as putting all the prepositions in English text in bold font.=09-- Rob Pike in "Notes on Programming in C" On Sunday 05 October 2008 19:10:49 Thorsten Glaser wrote: > Hello all, > > I found and fixed a rather interesting bug in ccache=E2=80=99s x_realloc() > function, which tries to read out too many bytes from the _old_ > storage after allocating the new storage. The obvious fix is to > make it use realloc(3) instead. > > Interestingly enough, there is an =E2=80=9Cif (ptr)=E2=80=9D in the (orig= inal) code > which is always true, since the =E2=80=9Cif (!ptr)=E2=80=9D case was alre= ady handled=E2=80=A6 > seems as if the author of that code did not know what he was doing. > > Effects of the bug: > =E2=80=A2 confirmed: segfault abortion (DoS) > =E2=80=A2 don=E2=80=99t know if it can lead to data corruption, don=E2=80= =99t think so > > Fix: http://www.mirbsd.org/cvs.cgi/ports/devel/ccache/patches/patch-util_c > > bye, > //mirabilos, who first thought his memcpy(3) was broken=E2=80=A6 I think the intent of x_realloc is to not rely on realloc being available o= n=20 target OS. If you're going to use realloc anyway, then why not #if 0=20 x_realloc and compile with -Dx_realloc=3Drealloc. @upstream: Thing is, userspace realloc cannot determine sizeof(old), without digging i= nto=20 target OS's allocation algorithm (i.e. what lib/libc/stdlib/malloc.c:irall= oc=20 does on FreeBSD), so x_realloc should really be prototyped as void *x_realloc(void *old, size_t old, size_t new); and act accordingly. =2D-=20 Mel Mel dixit: >I think the intent of x_realloc is to not rely on realloc being available = on=20 >target OS. What OS does not have realloc? oO >@upstream: >Thing is, userspace realloc cannot determine sizeof(old), without digging = into=20 >target OS's allocation algorithm (i.e. what lib/libc/stdlib/malloc.c:iral= loc=20 >does on FreeBSD), so x_realloc should really be prototyped as >void *x_realloc(void *old, size_t old, size_t new); Yes, indeed. bye, //mirabilos --=20 [...] if maybe ext3fs wasn't a better pick, or jfs, or maybe reiserfs, oh b= ut what about xfs, and if only i had waited until reiser4 was ready... in the = be- ginning, there was ffs, and in the middle, there was ffs, and at the end, t= here was still ffs, and the sys admins knew it was good. :) -- Ted Unangst =C3= =BCber *fs State Changed From-To: open->closed Committed, Thanks! ahze 2008-11-24 03:32:46 UTC
FreeBSD ports repository
Modified files:
devel/ccache Makefile
Added files:
devel/ccache/files patch-util.c
Log:
Fix a bug in x_realloc() which tries to read out too many bytes from the old storage after allocating new storage.
PR: ports/127639
Submitted by: Thorsten Glaser <tg@mirbsd.de> (MirBSD project)
Reported by: Mel <mel.xyzzy@rachie.is-a-geek.net>
Revision Changes Path
1.44 +1 -1 ports/devel/ccache/Makefile
1.1 +25 -0 ports/devel/ccache/files/patch-util.c (new)
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
|
util.c: 184 this is like realloc() but dies if the malloc fails 185 */ 186 void *x_realloc(void *ptr, size_t size) 187 { 188 void *p2; 189 if (!ptr) return x_malloc(size); 190 p2 = malloc(size); 191 if (!p2) { 192 fatal("out of memory in x_realloc"); 193 } 194 if (ptr) { 195 memcpy(p2, ptr, size); 196 free(ptr); 197 } 198 return p2; 199 } args.c: 38 void args_add(ARGS *args, const char *s) 39 { 40 args->argv = (char**)x_realloc(args->argv, (args->argc + 2) * sizeof(char *)); 41 args->argv[args->argc] = x_strdup(s); 42 args->argc++; 43 args->argv[args->argc] = NULL; 44 } Line 195 copies newsize of oldpointer to new pointer which can produce the following backtrace: (gdb) bt #0 0x0000000800816b86 in memcpy () from /lib/libc.so.6 #1 0x0000000000403fec in x_realloc (ptr=0x514800, size=2056) at util.c:195 #2 0x0000000000404512 in args_add (args=0x512040, s=0x7fffffffe2c3 "p12_key.So") at args.c:40 #3 0x00000000004045a1 in args_init (init_argc=455, init_args=0x7fffffffcf20) at args.c:32 #4 0x0000000000402a14 in main (argc=455, argv=0x7fffffffc720) at ccache.c:564 Fix: The following works around the problem by using reallocf, instead of x_malloc, however, the root of the problem is likely elsewhere. How-To-Repeat: I can't reproduce this using a test like this: ln -s ccache cc ./cc -L/usr/lib -shared `jot -w 'file%04u.So' 452 1 452` However, the following reproduces the bug reliably: #!/bin/sh SRCDIR=${SRCDIR:="/usr/src"} cd ${SRCDIR}/secure/lib/libcrypto rm -f `make -V .OBJDIR`/libcrypto.so.4 cd ${SRCDIR} make everything