From patchwork Tue Dec 25 20:29:06 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 1018440 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-98764-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=deneb.enyo.de 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 43PSTp29lXz9s9G for ; Wed, 26 Dec 2018 07:33:00 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-type; q=dns; s=default; b=IGTKLsC8OAriq4if MZv21S+joqZA6mu5YwgAjuCvNvjSk6dsE4SjS9mUEnUhxrMqjtq/eWyq0czo8i+F pnq6SC5Px3YGf/aQrl/XcUzpzCYV8D93TbJ3Es8gaDnBdNrRyVT/6LMiHOujkJ2h v6nJ8eMP3L5TGoDOy9wxF0LP5tQ= 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:from:to:cc:subject:date:message-id :mime-version:content-type; s=default; bh=nhbKhaZ4++G713rvUsu+Oz 2Kqes=; b=ZObDJyGna9Ug+p0Yd2zo+/c6FbHyrLaGOyjoQ+BsrKXDcjpOc2bY7O W3K9uT7YdrgqmaDV25a8VU1na+V/ERM6okLLlW3IrE1X0fj0B4ZQGkXoWE8PmRB9 OJZrDFX6dd1vYnnGYhW/HjF6lhTu1nlOj8ohkZZBE8VB0R4bt2LzU= Received: (qmail 68187 invoked by alias); 25 Dec 2018 20:32: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 68173 invoked by uid 89); 25 Dec 2018 20:32:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY autolearn=ham version=3.3.2 spammy=sizes, sk:perform, beyond, H*Ad:U*mail X-HELO: albireo.enyo.de From: Florian Weimer To: libc-alpha@sourceware.org Cc: mail@nh2.me Subject: [PATCH] malloc: Always call memcpy in _int_realloc [BZ #24027] Date: Tue, 25 Dec 2018 21:29:06 +0100 Message-ID: <87sgyluz8t.fsf@mid.deneb.enyo.de> MIME-Version: 1.0 This patch removes the custom memcpy implementation from _int_realloc for small chunk sizes. ncopies has the wrong type and could thus result in too few elements being copied, so removing this code fixes bug 24027. I don't think the inlining is performance-critical because this code is used when the realloc results in an _int_malloc, copy, and _int_free, so even for small allocations there is quote some overhead beyond the copy itself. I'm not sure if this warrants tracking as a security bug. Looking at the code, the problem could be trigger in a default configuration if mremap fails and a subsequent mmap succeeds. 2018-12-23 Florian Weimer [BZ #24027] * malloc/malloc.c (_int_realloc): Always call memcpy for the copying operation. diff --git a/malloc/malloc.c b/malloc/malloc.c index c33709e966..0a20a6054c 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -4581,11 +4581,6 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize, mchunkptr remainder; /* extra space at end of newp */ unsigned long remainder_size; /* its size */ - unsigned long copysize; /* bytes to copy */ - unsigned int ncopies; /* INTERNAL_SIZE_T words to copy */ - INTERNAL_SIZE_T* s; /* copy source */ - INTERNAL_SIZE_T* d; /* copy destination */ - /* oldmem size */ if (__builtin_expect (chunksize_nomask (oldp) <= 2 * SIZE_SZ, 0) || __builtin_expect (oldsize >= av->system_mem, 0)) @@ -4653,43 +4648,7 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize, } else { - /* - Unroll copy of <= 36 bytes (72 if 8byte sizes) - We know that contents have an odd number of - INTERNAL_SIZE_T-sized words; minimally 3. - */ - - copysize = oldsize - SIZE_SZ; - s = (INTERNAL_SIZE_T *) (chunk2mem (oldp)); - d = (INTERNAL_SIZE_T *) (newmem); - ncopies = copysize / sizeof (INTERNAL_SIZE_T); - assert (ncopies >= 3); - - if (ncopies > 9) - memcpy (d, s, copysize); - - else - { - *(d + 0) = *(s + 0); - *(d + 1) = *(s + 1); - *(d + 2) = *(s + 2); - if (ncopies > 4) - { - *(d + 3) = *(s + 3); - *(d + 4) = *(s + 4); - if (ncopies > 6) - { - *(d + 5) = *(s + 5); - *(d + 6) = *(s + 6); - if (ncopies > 8) - { - *(d + 7) = *(s + 7); - *(d + 8) = *(s + 8); - } - } - } - } - + memcpy (newmem, chunk2mem (oldp), oldsize - SIZE_SZ); _int_free (av, oldp, 1); check_inuse_chunk (av, newp); return chunk2mem (newp);