From patchwork Wed Apr 8 15:41:34 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Lemke X-Patchwork-Id: 459335 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 73C431401B1 for ; Thu, 9 Apr 2015 01:41:47 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass reason="1024-bit key; unprotected key" header.d=sourceware.org header.i=@sourceware.org header.b=HUMDENy5; dkim-adsp=none (unprotected policy); dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type; q=dns; s=default; b= iEGzE5V3FUUtlR6AfclEKq2Q12CBslqojRANoCIR5cvGoDqx6qTIpvPXPN6W+jtJ 4PVXs2KDFJ7r3iwv4wT9Y5NCoYV1w7I+OZDv/tJ0CrkMJELV+Vo18MBc9WKxqFpb mi1FaXMA9xiIJefiW1P9NTzz+K0oeymGo8KJ4wJxHqE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type; s=default; bh=ZHug 9CEI8Opi4yklKlHbVLzcKTk=; b=HUMDENy5ASi9CeeRZi4CRKwWQqpIYTiR1lkQ ew1e+afqhqWAXuIilbtve3WKPOoKicOcvylrgTiK/L+lVipgTFyhhXLWslKlRZKW Jr5JrrU96Y5Q78FCp8Z6UlB9ZNkWSj6PzeFFJVe1jGcnomKoms4P8c/W5ljaMDCN GdbymGE= Received: (qmail 66534 invoked by alias); 8 Apr 2015 15:41:41 -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 66524 invoked by uid 89); 8 Apr 2015 15:41:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Message-ID: <55254C2E.4040504@codesourcery.com> Date: Wed, 8 Apr 2015 11:41:34 -0400 From: James Lemke User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: libc-alpha , Mike Frysinger CC: Andreas Schwab , Will Newton Subject: Re: [PATCH] fix to malloc checking References: <5462592E.9050301@codesourcery.com> <5474FA02.6020701@mentor.com> <54762A68.5050801@codesourcery.com> <54D26B54.9050707@codesourcery.com> <20150302030859.GN19363@vapier> <550082A3.2010503@codesourcery.com> <20150405030555.GH16816@vapier> <5523DF4B.9070407@codesourcery.com> <20150408031058.GT16816@vapier> In-Reply-To: <20150408031058.GT16816@vapier> On 04/07/2015 11:10 PM, Mike Frysinger wrote: >> OK to commit with that change? > probably ? :) please post the latest version. The updated patch is attached. OK to commit? 2015-04-08 James Lemke [BZ #17581] * malloc/hooks.c (magicbyte): Convert to a function and avoid returning 0x01. (mem2mem_check): Avoid using a length byte equal to the magic byte. (mem2chunk_check): Fix unsigned comparisons to zero. Hoist defs of sz and magic. diff --git a/malloc/hooks.c b/malloc/hooks.c index 0c4816f..d2c64e6 100644 --- a/malloc/hooks.c +++ b/malloc/hooks.c @@ -88,11 +88,22 @@ __malloc_check_init (void) overruns. The goal here is to avoid obscure crashes due to invalid usage, unlike in the MALLOC_DEBUG code. */ -#define MAGICBYTE(p) ((((size_t) p >> 3) ^ ((size_t) p >> 11)) & 0xFF) +static unsigned char +magicbyte (const void *p) +{ + unsigned char magic; + + magic = (((uintptr_t) p >> 3) ^ ((uintptr_t) p >> 11)) & 0xFF; + /* Do not return 1. See the comment in mem2mem_check(). */ + if (magic == 1) + ++magic; + return magic; +} + -/* 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 +/* 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. 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. */ @@ -101,7 +112,7 @@ malloc_check_get_size (mchunkptr p) { size_t size; unsigned char c; - unsigned char magic = MAGICBYTE (p); + unsigned char magic = magicbyte (p); assert (using_malloc_checking == 1); @@ -122,32 +133,38 @@ malloc_check_get_size (mchunkptr p) } /* Instrument a chunk with overrun detector byte(s) and convert it - into a user pointer with requested size sz. */ + into a user pointer with requested size req_sz. */ static void * internal_function -mem2mem_check (void *ptr, size_t sz) +mem2mem_check (void *ptr, size_t req_sz) { mchunkptr p; unsigned char *m_ptr = ptr; - size_t i; + size_t max_sz, block_sz, i; + unsigned char magic; 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) + magic = magicbyte (p); + max_sz = chunksize (p) - 2 * SIZE_SZ; + if (!chunk_is_mmapped (p)) + max_sz += SIZE_SZ; + for (i = max_sz - 1; i > req_sz; i -= block_sz) { - if (i - sz < 0x100) - { - m_ptr[i] = (unsigned char) (i - sz); - break; - } - m_ptr[i] = 0xFF; + block_sz = i - req_sz; + if (block_sz > 0xff) + block_sz = 0xff; + /* Don't allow the magic byte to appear in the chain of length bytes. + For the following to work, magicbyte() cannot return 0x01. */ + if (block_sz == magic) + --block_sz; + + m_ptr[i] = (unsigned char) block_sz; } - m_ptr[sz] = MAGICBYTE (p); + m_ptr[req_sz] = magic; return (void *) m_ptr; } @@ -166,11 +183,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,10 +198,9 @@ 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) { - if (c <= 0 || sz < (c + 2 * SIZE_SZ)) + if (c == 0 || sz < (c + 2 * SIZE_SZ)) return NULL; } } @@ -201,13 +218,12 @@ 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) { - if (c <= 0 || sz < (c + 2 * SIZE_SZ)) + if (c == 0 || sz < (c + 2 * SIZE_SZ)) return NULL; } }