From db6b071a97893d5c7bf34e7fb171a0b710ea736d Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Wed, 3 Dec 2014 19:50:48 +0000 Subject: [PATCH] Fix memory access problems exposed by fuzzed binaries. PR binutils/17512 * objdump.c (free_debug_section): Reset the compress_status as well. * compress.c (bfd_get_full_section_contents): Fail if there are no section contents available when the compress_status is COMPRESS_SECTION_DONE. * libbfd.c (bfd_malloc): Refuse to allocate a negative size. (bfd_malloc2): Use bfd_malloc. (bfd_realloc): Refuse to reallocate a negative size. (bfd_realloc2): Use bfd_realloc. (bfd_realloc_or_free): Use bfd_realloc. (bfd_zmalloc): Use bfd_malloc. (bfd_zmalloc): Use bfd_malloc2. * opncls.c (bfd_alloc): Refuse to allocate a negative size. --- bfd/ChangeLog | 15 ++++++ bfd/compress.c | 2 + bfd/libbfd.c | 127 ++++++++++----------------------------------- bfd/opncls.c | 14 +++-- binutils/ChangeLog | 6 +++ binutils/objdump.c | 1 + 6 files changed, 61 insertions(+), 104 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 61a8b2b5ea4..65c72434225 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,18 @@ +2014-12-03 Nick Clifton + + PR binutils/17512 + * compress.c (bfd_get_full_section_contents): Fail if there are no + section contents available when the compress_status is + COMPRESS_SECTION_DONE. + * libbfd.c (bfd_malloc): Refuse to allocate a negative size. + (bfd_malloc2): Use bfd_malloc. + (bfd_realloc): Refuse to reallocate a negative size. + (bfd_realloc2): Use bfd_realloc. + (bfd_realloc_or_free): Use bfd_realloc. + (bfd_zmalloc): Use bfd_malloc. + (bfd_zmalloc): Use bfd_malloc2. + * opncls.c (bfd_alloc): Refuse to allocate a negative size. + 2014-12-03 H.J. Lu * elf64-x86-64.c (elf_x86_64_create_dynamic_sections): Reformat. diff --git a/bfd/compress.c b/bfd/compress.c index 3fcbd783ee4..a3a2f359627 100644 --- a/bfd/compress.c +++ b/bfd/compress.c @@ -244,6 +244,8 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr) #endif case COMPRESS_SECTION_DONE: + if (sec->contents == NULL) + return FALSE; if (p == NULL) { p = (bfd_byte *) bfd_malloc (sz); diff --git a/bfd/libbfd.c b/bfd/libbfd.c index 6352c9cf836..ade8e0fc833 100644 --- a/bfd/libbfd.c +++ b/bfd/libbfd.c @@ -171,15 +171,18 @@ void * bfd_malloc (bfd_size_type size) { void *ptr; + size_t sz = (size_t) size; - if (size != (size_t) size) + if (size != sz + /* This is to pacify memory checkers like valgrind. */ + || ((signed long) sz) < 0) { bfd_set_error (bfd_error_no_memory); return NULL; } - ptr = malloc ((size_t) size); - if (ptr == NULL && (size_t) size != 0) + ptr = malloc (sz); + if (ptr == NULL && sz != 0) bfd_set_error (bfd_error_no_memory); return ptr; @@ -190,8 +193,6 @@ bfd_malloc (bfd_size_type size) void * bfd_malloc2 (bfd_size_type nmemb, bfd_size_type size) { - void *ptr; - if ((nmemb | size) >= HALF_BFD_SIZE_TYPE && size != 0 && nmemb > ~(bfd_size_type) 0 / size) @@ -200,19 +201,7 @@ bfd_malloc2 (bfd_size_type nmemb, bfd_size_type size) return NULL; } - size *= nmemb; - - if (size != (size_t) size) - { - bfd_set_error (bfd_error_no_memory); - return NULL; - } - - ptr = malloc ((size_t) size); - if (ptr == NULL && (size_t) size != 0) - bfd_set_error (bfd_error_no_memory); - - return ptr; + return bfd_malloc (size * nmemb); } /* Reallocate memory using realloc. */ @@ -221,19 +210,22 @@ void * bfd_realloc (void *ptr, bfd_size_type size) { void *ret; + size_t sz = (size_t) size; + + if (ptr == NULL) + return bfd_malloc (size); - if (size != (size_t) size) + if (size != sz + /* This is to pacify memory checkers like valgrind. */ + || ((signed long) sz) < 0) { bfd_set_error (bfd_error_no_memory); return NULL; } - if (ptr == NULL) - ret = malloc ((size_t) size); - else - ret = realloc (ptr, (size_t) size); + ret = realloc (ptr, sz); - if (ret == NULL && (size_t) size != 0) + if (ret == NULL && sz != 0) bfd_set_error (bfd_error_no_memory); return ret; @@ -244,8 +236,6 @@ bfd_realloc (void *ptr, bfd_size_type size) void * bfd_realloc2 (void *ptr, bfd_size_type nmemb, bfd_size_type size) { - void *ret; - if ((nmemb | size) >= HALF_BFD_SIZE_TYPE && size != 0 && nmemb > ~(bfd_size_type) 0 / size) @@ -254,23 +244,7 @@ bfd_realloc2 (void *ptr, bfd_size_type nmemb, bfd_size_type size) return NULL; } - size *= nmemb; - - if (size != (size_t) size) - { - bfd_set_error (bfd_error_no_memory); - return NULL; - } - - if (ptr == NULL) - ret = malloc ((size_t) size); - else - ret = realloc (ptr, (size_t) size); - - if (ret == NULL && (size_t) size != 0) - bfd_set_error (bfd_error_no_memory); - - return ret; + return bfd_realloc (ptr, size * nmemb); } /* Reallocate memory using realloc. @@ -279,24 +253,10 @@ bfd_realloc2 (void *ptr, bfd_size_type nmemb, bfd_size_type size) void * bfd_realloc_or_free (void *ptr, bfd_size_type size) { - size_t amount = (size_t) size; - void *ret; + void *ret = bfd_realloc (ptr, size); - if (size != amount) - ret = NULL; - else if (ptr == NULL) - ret = malloc (amount); - else - ret = realloc (ptr, amount); - - if (ret == NULL) - { - if (amount > 0) - bfd_set_error (bfd_error_no_memory); - - if (ptr != NULL) - free (ptr); - } + if (ret == NULL && ptr != NULL) + free (ptr); return ret; } @@ -306,23 +266,10 @@ bfd_realloc_or_free (void *ptr, bfd_size_type size) void * bfd_zmalloc (bfd_size_type size) { - void *ptr; + void *ptr = bfd_malloc (size); - if (size != (size_t) size) - { - bfd_set_error (bfd_error_no_memory); - return NULL; - } - - ptr = malloc ((size_t) size); - - if ((size_t) size != 0) - { - if (ptr == NULL) - bfd_set_error (bfd_error_no_memory); - else - memset (ptr, 0, (size_t) size); - } + if (ptr != NULL && size > 0) + memset (ptr, 0, (size_t) size); return ptr; } @@ -333,32 +280,14 @@ bfd_zmalloc (bfd_size_type size) void * bfd_zmalloc2 (bfd_size_type nmemb, bfd_size_type size) { - void *ptr; + void *ptr = bfd_malloc2 (nmemb, size); - if ((nmemb | size) >= HALF_BFD_SIZE_TYPE - && size != 0 - && nmemb > ~(bfd_size_type) 0 / size) + if (ptr != NULL) { - bfd_set_error (bfd_error_no_memory); - return NULL; - } + size_t sz = nmemb * size; - size *= nmemb; - - if (size != (size_t) size) - { - bfd_set_error (bfd_error_no_memory); - return NULL; - } - - ptr = malloc ((size_t) size); - - if ((size_t) size != 0) - { - if (ptr == NULL) - bfd_set_error (bfd_error_no_memory); - else - memset (ptr, 0, (size_t) size); + if (sz > 0) + memset (ptr, 0, sz); } return ptr; diff --git a/bfd/opncls.c b/bfd/opncls.c index 75af627d293..404b94422a8 100644 --- a/bfd/opncls.c +++ b/bfd/opncls.c @@ -940,15 +940,19 @@ bfd_alloc (bfd *abfd, bfd_size_type size) unsigned long ul_size = (unsigned long) size; if (size != ul_size - /* A small negative size can result in objalloc_alloc allocating just - 1 byte of memory, but the caller will be expecting more. So catch - this case here. */ - || (size != 0 && (((ul_size + OBJALLOC_ALIGN - 1) &~ (OBJALLOC_ALIGN - 1)) == 0))) + /* Note - although objalloc_alloc takes an unsigned long as its + argument, internally the size is treated as a signed long. This can + lead to problems where, for example, a request to allocate -1 bytes + can result in just 1 byte being allocated, rather than + ((unsigned long) -1) bytes. Also memory checkers will often + complain about attempts to allocate a negative amount of memory. + So to stop these problems we fail if the size is negative. */ + || ((signed long) ul_size) < 0) { bfd_set_error (bfd_error_no_memory); return NULL; } - + ret = objalloc_alloc ((struct objalloc *) abfd->memory, ul_size); if (ret == NULL) bfd_set_error (bfd_error_no_memory); diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 2d28e1c2818..8740dd2d116 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,9 @@ +2014-12-03 Nick Clifton + + PR binutils/17512 + * objdump.c (free_debug_section): Reset the compress_status as + well. + 2014-12-03 Nick Clifton PR binutils/17531 diff --git a/binutils/objdump.c b/binutils/objdump.c index b13bf00e699..b43d11171d5 100644 --- a/binutils/objdump.c +++ b/binutils/objdump.c @@ -2360,6 +2360,7 @@ free_debug_section (enum dwarf_section_display_enum debug) { sec->contents = NULL; sec->flags &= ~ SEC_IN_MEMORY; + sec->compress_status = COMPRESS_SECTION_NONE; } } -- 2.30.2