From: Paul Eggert Date: Thu, 7 Jul 2011 16:58:24 +0000 (-0700) Subject: * alloc.c: Integer signedness and overflow fixes. X-Git-Tag: emacs-pretest-24.0.90~104^2~152^2~174 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=903fe15d9deb28a72075c39dfd6003a2ff1af134;p=emacs.git * alloc.c: Integer signedness and overflow fixes. Do not impose an arbitrary 32-bit limit on malloc sizes when debugging. (__malloc_size_t): Default to size_t, not to int. (pure_size, pure_bytes_used_before_overflow, stack_copy_size) (Fgarbage_collect, mark_object_loop_halt, mark_object): Prefer ptrdiff_t to size_t when either would do, as we prefer signed integers. (XMALLOC_OVERRUN_CHECK_OVERHEAD): New macro. (xmalloc_overrun_check_header, xmalloc_overrun_check_trailer): Now const. Initialize with values that are in range even if char is signed. (XMALLOC_PUT_SIZE, XMALLOC_GET_SIZE): Remove, replacing with ... (xmalloc_put_size, xmalloc_get_size): New functions. All uses changed. These functions do the right thing with sizes > 2**32. (check_depth): Now ptrdiff_t, not int. (overrun_check_malloc, overrun_check_realloc, overrun_check_free): Adjust to new way of storing sizes. Check for size overflow bugs in rest of code. (STRING_BYTES_MAX): Adjust to new overheads. The old code was slightly wrong anyway, as it missed one instance of XMALLOC_OVERRUN_CHECK_OVERHEAD. (refill_memory_reserve): Omit needless cast to size_t. (mark_object_loop_halt): Mark as externally visible. --- diff --git a/src/ChangeLog b/src/ChangeLog index 43d449b459b..f2c318fa84a 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,29 @@ 2011-07-07 Paul Eggert + * alloc.c: Integer signedness and overflow fixes. + Do not impose an arbitrary 32-bit limit on malloc sizes when debugging. + (__malloc_size_t): Default to size_t, not to int. + (pure_size, pure_bytes_used_before_overflow, stack_copy_size) + (Fgarbage_collect, mark_object_loop_halt, mark_object): + Prefer ptrdiff_t to size_t when either would do, as we prefer + signed integers. + (XMALLOC_OVERRUN_CHECK_OVERHEAD): New macro. + (xmalloc_overrun_check_header, xmalloc_overrun_check_trailer): + Now const. Initialize with values that are in range even if char + is signed. + (XMALLOC_PUT_SIZE, XMALLOC_GET_SIZE): Remove, replacing with ... + (xmalloc_put_size, xmalloc_get_size): New functions. All uses changed. + These functions do the right thing with sizes > 2**32. + (check_depth): Now ptrdiff_t, not int. + (overrun_check_malloc, overrun_check_realloc, overrun_check_free): + Adjust to new way of storing sizes. Check for size overflow bugs + in rest of code. + (STRING_BYTES_MAX): Adjust to new overheads. The old code was + slightly wrong anyway, as it missed one instance of + XMALLOC_OVERRUN_CHECK_OVERHEAD. + (refill_memory_reserve): Omit needless cast to size_t. + (mark_object_loop_halt): Mark as externally visible. + * xselect.c: Integer signedness and overflow fixes. (Fx_register_dnd_atom, x_handle_dnd_message): Use ptrdiff_t, not size_t, since we prefer signed. diff --git a/src/alloc.c b/src/alloc.c index f679787e95c..5cdd5af9ecf 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -70,7 +70,7 @@ extern POINTER_TYPE *sbrk (); #include /* malloc.h #defines this as size_t, at least in glibc2. */ #ifndef __malloc_size_t -#define __malloc_size_t int +#define __malloc_size_t size_t #endif /* Specify maximum number of areas to mmap. It would be nice to use a @@ -214,12 +214,12 @@ EMACS_INT pure[(PURESIZE + sizeof (EMACS_INT) - 1) / sizeof (EMACS_INT)] = {1,}; /* Pointer to the pure area, and its size. */ static char *purebeg; -static size_t pure_size; +static ptrdiff_t pure_size; /* Number of bytes of pure storage used before pure storage overflowed. If this is non-zero, this implies that an overflow occurred. */ -static size_t pure_bytes_used_before_overflow; +static ptrdiff_t pure_bytes_used_before_overflow; /* Value is non-zero if P points into pure space. */ @@ -252,7 +252,7 @@ const char *pending_malloc_warning; #if MAX_SAVE_STACK > 0 static char *stack_copy; -static size_t stack_copy_size; +static ptrdiff_t stack_copy_size; #endif /* Non-zero means ignore malloc warnings. Set during initialization. @@ -486,14 +486,15 @@ buffer_memory_full (EMACS_INT nbytes) #ifndef XMALLOC_OVERRUN_CHECK -#define XMALLOC_OVERRUN_CHECK_SIZE 0 +#define XMALLOC_OVERRUN_CHECK_OVERHEAD 0 #else -/* Check for overrun in malloc'ed buffers by wrapping a 16 byte header - and a 16 byte trailer around each block. +/* Check for overrun in malloc'ed buffers by wrapping a header and trailer + around each block. - The header consists of 12 fixed bytes + a 4 byte integer contaning the - original block size, while the trailer consists of 16 fixed bytes. + The header consists of 16 fixed bytes followed by sizeof (size_t) bytes + containing the original block size in little-endian order, + while the trailer consists of 16 fixed bytes. The header is used to detect whether this block has been allocated through these functions -- as it seems that some low-level libc @@ -502,31 +503,47 @@ buffer_memory_full (EMACS_INT nbytes) #define XMALLOC_OVERRUN_CHECK_SIZE 16 +#define XMALLOC_OVERRUN_CHECK_OVERHEAD \ + (2 * XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t)) -static char xmalloc_overrun_check_header[XMALLOC_OVERRUN_CHECK_SIZE-4] = - { 0x9a, 0x9b, 0xae, 0xaf, - 0xbf, 0xbe, 0xce, 0xcf, - 0xea, 0xeb, 0xec, 0xed }; +static char const xmalloc_overrun_check_header[XMALLOC_OVERRUN_CHECK_SIZE] = + { '\x9a', '\x9b', '\xae', '\xaf', + '\xbf', '\xbe', '\xce', '\xcf', + '\xea', '\xeb', '\xec', '\xed', + '\xdf', '\xde', '\x9c', '\x9d' }; -static char xmalloc_overrun_check_trailer[XMALLOC_OVERRUN_CHECK_SIZE] = - { 0xaa, 0xab, 0xac, 0xad, - 0xba, 0xbb, 0xbc, 0xbd, - 0xca, 0xcb, 0xcc, 0xcd, - 0xda, 0xdb, 0xdc, 0xdd }; +static char const xmalloc_overrun_check_trailer[XMALLOC_OVERRUN_CHECK_SIZE] = + { '\xaa', '\xab', '\xac', '\xad', + '\xba', '\xbb', '\xbc', '\xbd', + '\xca', '\xcb', '\xcc', '\xcd', + '\xda', '\xdb', '\xdc', '\xdd' }; -/* Macros to insert and extract the block size in the header. */ +/* Insert and extract the block size in the header. */ -#define XMALLOC_PUT_SIZE(ptr, size) \ - (ptr[-1] = (size & 0xff), \ - ptr[-2] = ((size >> 8) & 0xff), \ - ptr[-3] = ((size >> 16) & 0xff), \ - ptr[-4] = ((size >> 24) & 0xff)) +static void +xmalloc_put_size (unsigned char *ptr, size_t size) +{ + int i; + for (i = 0; i < sizeof (size_t); i++) + { + *--ptr = size & (1 << CHAR_BIT) - 1; + size >>= CHAR_BIT; + } +} -#define XMALLOC_GET_SIZE(ptr) \ - (size_t)((unsigned)(ptr[-1]) | \ - ((unsigned)(ptr[-2]) << 8) | \ - ((unsigned)(ptr[-3]) << 16) | \ - ((unsigned)(ptr[-4]) << 24)) +static size_t +xmalloc_get_size (unsigned char *ptr) +{ + size_t size = 0; + int i; + ptr -= sizeof (size_t); + for (i = 0; i < sizeof (size_t); i++) + { + size <<= CHAR_BIT; + size += *ptr++; + } + return size; +} /* The call depth in overrun_check functions. For example, this might happen: @@ -545,10 +562,10 @@ static char xmalloc_overrun_check_trailer[XMALLOC_OVERRUN_CHECK_SIZE] = xfree(10032) overrun_check_free(10032) - decrease overhed + decrease overhead free(10016) <- crash, because 10000 is the original pointer. */ -static int check_depth; +static ptrdiff_t check_depth; /* Like malloc, but wraps allocated block with header and trailer. */ @@ -556,15 +573,16 @@ static POINTER_TYPE * overrun_check_malloc (size_t size) { register unsigned char *val; - size_t overhead = ++check_depth == 1 ? XMALLOC_OVERRUN_CHECK_SIZE*2 : 0; + int overhead = ++check_depth == 1 ? XMALLOC_OVERRUN_CHECK_OVERHEAD : 0; + if (SIZE_MAX - overhead < size) + abort (); val = (unsigned char *) malloc (size + overhead); if (val && check_depth == 1) { - memcpy (val, xmalloc_overrun_check_header, - XMALLOC_OVERRUN_CHECK_SIZE - 4); - val += XMALLOC_OVERRUN_CHECK_SIZE; - XMALLOC_PUT_SIZE(val, size); + memcpy (val, xmalloc_overrun_check_header, XMALLOC_OVERRUN_CHECK_SIZE); + val += XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t); + xmalloc_put_size (val, size); memcpy (val + size, xmalloc_overrun_check_trailer, XMALLOC_OVERRUN_CHECK_SIZE); } @@ -580,31 +598,32 @@ static POINTER_TYPE * overrun_check_realloc (POINTER_TYPE *block, size_t size) { register unsigned char *val = (unsigned char *) block; - size_t overhead = ++check_depth == 1 ? XMALLOC_OVERRUN_CHECK_SIZE*2 : 0; + int overhead = ++check_depth == 1 ? XMALLOC_OVERRUN_CHECK_OVERHEAD : 0; + if (SIZE_MAX - overhead < size) + abort (); if (val && check_depth == 1 && memcmp (xmalloc_overrun_check_header, - val - XMALLOC_OVERRUN_CHECK_SIZE, - XMALLOC_OVERRUN_CHECK_SIZE - 4) == 0) + val - XMALLOC_OVERRUN_CHECK_SIZE - sizeof (size_t), + XMALLOC_OVERRUN_CHECK_SIZE) == 0) { - size_t osize = XMALLOC_GET_SIZE (val); + size_t osize = xmalloc_get_size (val); if (memcmp (xmalloc_overrun_check_trailer, val + osize, XMALLOC_OVERRUN_CHECK_SIZE)) abort (); memset (val + osize, 0, XMALLOC_OVERRUN_CHECK_SIZE); - val -= XMALLOC_OVERRUN_CHECK_SIZE; - memset (val, 0, XMALLOC_OVERRUN_CHECK_SIZE); + val -= XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t); + memset (val, 0, XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t)); } val = (unsigned char *) realloc ((POINTER_TYPE *)val, size + overhead); if (val && check_depth == 1) { - memcpy (val, xmalloc_overrun_check_header, - XMALLOC_OVERRUN_CHECK_SIZE - 4); - val += XMALLOC_OVERRUN_CHECK_SIZE; - XMALLOC_PUT_SIZE(val, size); + memcpy (val, xmalloc_overrun_check_header, XMALLOC_OVERRUN_CHECK_SIZE); + val += XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t); + xmalloc_put_size (val, size); memcpy (val + size, xmalloc_overrun_check_trailer, XMALLOC_OVERRUN_CHECK_SIZE); } @@ -623,20 +642,20 @@ overrun_check_free (POINTER_TYPE *block) if (val && check_depth == 1 && memcmp (xmalloc_overrun_check_header, - val - XMALLOC_OVERRUN_CHECK_SIZE, - XMALLOC_OVERRUN_CHECK_SIZE - 4) == 0) + val - XMALLOC_OVERRUN_CHECK_SIZE - sizeof (size_t), + XMALLOC_OVERRUN_CHECK_SIZE) == 0) { - size_t osize = XMALLOC_GET_SIZE (val); + size_t osize = xmalloc_get_size (val); if (memcmp (xmalloc_overrun_check_trailer, val + osize, XMALLOC_OVERRUN_CHECK_SIZE)) abort (); #ifdef XMALLOC_CLEAR_FREE_MEMORY - val -= XMALLOC_OVERRUN_CHECK_SIZE; - memset (val, 0xff, osize + XMALLOC_OVERRUN_CHECK_SIZE*2); + val -= XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t); + memset (val, 0xff, osize + XMALLOC_OVERRUN_CHECK_OVERHEAD); #else memset (val + osize, 0, XMALLOC_OVERRUN_CHECK_SIZE); - val -= XMALLOC_OVERRUN_CHECK_SIZE; - memset (val, 0, XMALLOC_OVERRUN_CHECK_SIZE); + val -= XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t); + memset (val, 0, XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t)); #endif } @@ -1661,7 +1680,8 @@ static char const string_overrun_cookie[GC_STRING_OVERRUN_COOKIE_SIZE] = calculating a value to be passed to malloc. */ #define STRING_BYTES_MAX \ min (STRING_BYTES_BOUND, \ - ((SIZE_MAX - XMALLOC_OVERRUN_CHECK_SIZE - GC_STRING_EXTRA \ + ((SIZE_MAX - XMALLOC_OVERRUN_CHECK_OVERHEAD \ + - GC_STRING_EXTRA \ - offsetof (struct sblock, first_data) \ - SDATA_DATA_OFFSET) \ & ~(sizeof (EMACS_INT) - 1))) @@ -3320,7 +3340,7 @@ refill_memory_reserve (void) { #ifndef SYSTEM_MALLOC if (spare_memory[0] == 0) - spare_memory[0] = (char *) malloc ((size_t) SPARE_MEMORY); + spare_memory[0] = (char *) malloc (SPARE_MEMORY); if (spare_memory[1] == 0) spare_memory[1] = (char *) lisp_align_malloc (sizeof (struct cons_block), MEM_TYPE_CONS); @@ -4922,7 +4942,7 @@ returns nil, because real GC can't be done. */) if (NILP (Vpurify_flag)) { char *stack; - size_t stack_size; + ptrdiff_t stack_size; if (&stack_top_variable < stack_bottom) { stack = &stack_top_variable; @@ -5233,7 +5253,7 @@ static int last_marked_index; links of a list, in mark_object. In debugging, the call to abort will hit a breakpoint. Normally this is zero and the check never goes off. */ -static size_t mark_object_loop_halt; +ptrdiff_t mark_object_loop_halt EXTERNALLY_VISIBLE; static void mark_vectorlike (struct Lisp_Vector *ptr) @@ -5290,7 +5310,7 @@ mark_object (Lisp_Object arg) void *po; struct mem_node *m; #endif - size_t cdr_count = 0; + ptrdiff_t cdr_count = 0; loop: