From patchwork Tue Nov 18 20:24:46 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Lemke X-Patchwork-Id: 3795 Received: (qmail 31921 invoked by alias); 18 Nov 2014 20:24:53 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 31911 invoked by uid 89); 18 Nov 2014 20:24:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Message-ID: <546BAB0E.6030702@mentor.com> Date: Tue, 18 Nov 2014 15:24:46 -0500 From: James Lemke User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Subject: Re: [PATCH] fix to malloc checking References: <5462592E.9050301@codesourcery.com> In-Reply-To: <5462592E.9050301@codesourcery.com> Ping.. OK to commit? The updated patch is attached. 2014-11-11 James Lemke [BZ #17581] * malloc/hooks.c (mem2mem_check): Add a terminator to the chain of checking blocks. (malloc_check_get_size): Use it here. (mem2chunk_check): Ditto. diff --git a/malloc/hooks.c b/malloc/hooks.c index 00ee6be..c18140d 100644 --- a/malloc/hooks.c +++ b/malloc/hooks.c @@ -90,31 +90,35 @@ __malloc_check_init (void) #define MAGICBYTE(p) ((((size_t) p >> 3) ^ ((size_t) p >> 11)) & 0xFF) -/* Visualize the chunk as being partitioned into blocks of 256 bytes from the - highest address of the chunk, downwards. The beginning of each block tells - us the size of the previous block, up to the actual size of the requested - memory. Our magic byte is right at the end of the requested size, so we - must reach it with this iteration, otherwise we have witnessed a memory - corruption. */ +/* Visualize the chunk as being partitioned into blocks of 255 bytes from the + highest address of the chunk, downwards. The end of each block tells us + the size of that block, up to the actual size of the requested memory. + The last block has a length of zero and is followed by the magic byte. + Our magic byte is right at the end of the requested size. If we don't + reach it with this iteration we have witnessed a memory corruption. */ static size_t malloc_check_get_size (mchunkptr p) { - size_t size; + size_t total_sz, size; unsigned char c; unsigned char magic = MAGICBYTE (p); assert (using_malloc_checking == 1); - for (size = chunksize (p) - 1 + (chunk_is_mmapped (p) ? 0 : SIZE_SZ); - (c = ((unsigned char *) p)[size]) != magic; + /* Validate the length-byte chain. */ + total_sz = chunksize (p) + (chunk_is_mmapped (p) ? 0 : SIZE_SZ); + for (size = total_sz - 1; + (c = ((unsigned char *) p)[size]) != 0; size -= c) { - if (c <= 0 || size < (c + 2 * SIZE_SZ)) - { - malloc_printerr (check_action, "malloc_check_get_size: memory corruption", - chunk2mem (p)); - return 0; - } + if (size <= c + 2 * SIZE_SZ) + break; + } + if ( c != 0 || ((unsigned char *)p)[--size] != magic) + { + malloc_printerr (check_action, "malloc_check_get_size: memory corruption", + chunk2mem (p)); + return 0; } /* chunk2mem size. */ @@ -130,22 +134,24 @@ mem2mem_check (void *ptr, size_t sz) { mchunkptr p; unsigned char *m_ptr = ptr; - size_t i; + size_t user_sz, block_sz, i; if (!ptr) return ptr; p = mem2chunk (ptr); - for (i = chunksize (p) - (chunk_is_mmapped (p) ? 2 * SIZE_SZ + 1 : SIZE_SZ + 1); - i > sz; - i -= 0xFF) + user_sz = chunksize (p) + (chunk_is_mmapped (p) ? 0 : SIZE_SZ); + user_sz -= 2 * SIZE_SZ; + for (i = user_sz - 1; i > sz; i -= block_sz) { - if (i - sz < 0x100) - { - m_ptr[i] = (unsigned char) (i - sz); - break; - } - m_ptr[i] = 0xFF; + block_sz = i - (sz + 1); + if (block_sz > 0xff) + block_sz = 0xff; + + m_ptr[i] = (unsigned char) block_sz; + + if (block_sz == 0) + break; } m_ptr[sz] = MAGICBYTE (p); return (void *) m_ptr; @@ -166,11 +172,12 @@ mem2chunk_check (void *mem, unsigned char **magic_p) return NULL; p = mem2chunk (mem); + sz = chunksize (p); + magic = MAGICBYTE (p); if (!chunk_is_mmapped (p)) { /* Must be a chunk in conventional heap memory. */ int contig = contiguous (&main_arena); - sz = chunksize (p); if ((contig && ((char *) p < mp_.sbrk_base || ((char *) p + sz) >= (mp_.sbrk_base + main_arena.system_mem))) || @@ -180,12 +187,13 @@ mem2chunk_check (void *mem, unsigned char **magic_p) next_chunk (prev_chunk (p)) != p))) return NULL; - magic = MAGICBYTE (p); - for (sz += SIZE_SZ - 1; (c = ((unsigned char *) p)[sz]) != magic; sz -= c) + for (sz += SIZE_SZ - 1; (c = ((unsigned char *) p)[sz]) != 0; sz -= c) { - if (c <= 0 || sz < (c + 2 * SIZE_SZ)) - return NULL; + if (sz <= c + 2 * SIZE_SZ) + break; } + if ( c != 0 || ((unsigned char *)p)[--sz] != magic) + return NULL; } else { @@ -201,15 +209,17 @@ mem2chunk_check (void *mem, unsigned char **magic_p) offset < 0x2000) || !chunk_is_mmapped (p) || (p->size & PREV_INUSE) || ((((unsigned long) p - p->prev_size) & page_mask) != 0) || - ((sz = chunksize (p)), ((p->prev_size + sz) & page_mask) != 0)) + ((p->prev_size + sz) & page_mask) != 0 + ) return NULL; - magic = MAGICBYTE (p); - for (sz -= 1; (c = ((unsigned char *) p)[sz]) != magic; sz -= c) + for (sz -= 1; (c = ((unsigned char *) p)[sz]) != 0; sz -= c) { - if (c <= 0 || sz < (c + 2 * SIZE_SZ)) - return NULL; + if (sz <= c + 2 * SIZE_SZ) + break; } + if ( c != 0 || ((unsigned char *)p)[--sz] != magic) + return NULL; } ((unsigned char *) p)[sz] ^= 0xFF; if (magic_p)