From patchwork Thu Jan 4 17:02:51 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arjun Shankar X-Patchwork-Id: 855729 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-88829-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="YsVr4eqs"; dkim-atps=neutral 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 3zCDdN01YTz9s7G for ; Fri, 5 Jan 2018 04:03:03 +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:date:from:to:subject:message-id:mime-version :content-type; q=dns; s=default; b=e6v8iu1pTpGwQgXYGwxRow+aWjg4G Y/r1TGnrfU6b4gc1RRoesXYJjVO/SpyiV1J5OSCZ+y7glAO+QNm0XEQ6kJ+OLT6Y 6KItl3fdJmfPi9gwa8oi1ClU5kht3rrmrsTF1Y6hub46ixNGEmmmvQ1pNYnZkBtB 2VFd0hZTaSos3I= 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:date:from:to:subject:message-id:mime-version :content-type; s=default; bh=7nkZgI9p9RY4y1UpEI1NWrncDWg=; b=YsV r4eqsTaxOojYVrsWpI/RSPgFheggfMIale4N+xhrimlfKS2+qt1JJmDGmpLgGaXz nQduLns6dXSrQdkU+lXp9BK/VYB/prHAyZ1JA5I4k94am7o/s15j4Q2e+ZX117CE tcXv0V4UrcmqneF28u4a+SbC9paZ+aMHsk8d0dXA= Received: (qmail 20697 invoked by alias); 4 Jan 2018 17:02:57 -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 20684 invoked by uid 89); 4 Jan 2018 17:02:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=UD:se, addressable, arjun X-HELO: aloka.lostca.se Date: Thu, 4 Jan 2018 17:02:51 +0000 From: Arjun Shankar To: libc-alpha@sourceware.org Subject: [PATCH] Fix integer overflows in internal memalign and malloc functions [BZ #22343] Message-ID: <20180104170250.GA72870@aloka.lostca.se> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.7.1 (2016-10-04) When posix_memalign is called with an alignment less than MALLOC_ALIGNMENT and a requested size close to SIZE_MAX, it falls back to malloc code (because the alignment of a block returned by malloc is sufficient to satisfy the call). In this case, an integer overflow in _int_malloc leads to posix_memalign incorrectly returning successfully. Upon fixing this and writing a somewhat thorough regression test, it was discovered that when posix_memalign is called with an alignment larger than MALLOC_ALIGNMENT (so it uses _int_memalign instead) and a requested size close to SIZE_MAX, a different integer overflow in _int_memalign leads to posix_memalign incorrectly returning successfully. Both integer overflows affect other memory allocation functions that use _int_malloc (one affected malloc in x86) or _int_memalign as well. This commit fixes both integer overflows. In addition to this, it adds a regression test to guard against false successful allocations by the following memory allocation functions when called with too-large allocation sizes and, where relevant, various valid alignments: malloc, realloc, calloc, reallocarray, memalign, posix_memalign, aligned_alloc, valloc, and pvalloc. ChangeLog: 2018-01-04 Arjun Shankar [BZ #22343] * malloc/malloc.c (checked_request2size): call REQUEST_OUT_OF_RANGE after padding. (_int_memalign): check for integer overflow before calling _int_malloc. * malloc/tst-malloc-too-large.c: New test. * malloc/Makefile: Add tst-malloc-too-large. --- Two notes: 1. This bug has been flagged as security relevant and needs a CVE ID. 2. Although this patch stands on its own, a previous version of the test was reviewed by Florian. I dropped it when committing the earlier (related) patch pending this new change. Here is that review: https://sourceware.org/ml/libc-alpha/2017-11/msg00325.html Since then, I have expanded the scope of the test a little. malloc/Makefile | 1 + malloc/malloc.c | 32 ++++-- malloc/tst-malloc-too-large.c | 253 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 8 deletions(-) create mode 100644 malloc/tst-malloc-too-large.c diff --git a/malloc/Makefile b/malloc/Makefile index 4266c2b66b..17873e67c4 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -36,6 +36,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-alloc_buffer \ tst-malloc-tcache-leak \ tst-malloc_info \ + tst-malloc-too-large \ tests-static := \ tst-interpose-static-nothread \ diff --git a/malloc/malloc.c b/malloc/malloc.c index 48106f9bd4..2bcf64cdb6 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -1224,14 +1224,23 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ MINSIZE : \ ((req) + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK) -/* Same, except also perform argument check */ - -#define checked_request2size(req, sz) \ - if (REQUEST_OUT_OF_RANGE (req)) { \ - __set_errno (ENOMEM); \ - return 0; \ - } \ - (sz) = request2size (req); +/* Same, except also perform an argument and result check. First, we check + that the padding done by request2size didn't result in an integer + overflow. Then we check (using REQUEST_OUT_OF_RANGE) that the resulting + size isn't so large that a later alignment would lead to another integer + overflow. */ +#define checked_request2size(req, sz) \ + do \ + { \ + (sz) = request2size (req); \ + if (((sz) < (req)) \ + || REQUEST_OUT_OF_RANGE (sz)) \ + { \ + __set_errno (ENOMEM); \ + return 0; \ + } \ + } \ + while (0) /* --------------- Physical chunk operations --------------- @@ -4672,6 +4681,13 @@ _int_memalign (mstate av, size_t alignment, size_t bytes) */ + /* Check for overflow. */ + if (nb > SIZE_MAX - alignment - MINSIZE) + { + __set_errno (ENOMEM); + return 0; + } + /* Call malloc with worst case padding to hit alignment. */ m = (char *) (_int_malloc (av, nb + alignment + MINSIZE)); diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c new file mode 100644 index 0000000000..705a07d850 --- /dev/null +++ b/malloc/tst-malloc-too-large.c @@ -0,0 +1,253 @@ +/* Test and verify that too-large memory allocations fail with ENOMEM. + Copyright (C) 2018 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* Bug 22375 reported a regression in malloc where if after malloc'ing then + free'ing a small block of memory, malloc is then called with a really + large size argument (close to SIZE_MAX): instead of returning NULL and + setting errno to ENOMEM, malloc incorrectly returns the previously + allocated block instead. Bug 22343 reported a similar case where + posix_memalign incorrectly returns successfully when called with an with + a really large size argument. + + Both of these were caused by integer overflows in the allocator when it + was trying to pad the requested size to allow for book-keeping or + alignment. This test guards against such bugs by repeatedly allocating + and freeing small blocks of memory then trying to allocate various block + sizes larger than the memory bus width of 64-bit targets, or almost + as large as SIZE_MAX on 32-bit targets supported by glibc. In each case, + it verifies that such impossibly large allocations correctly fail. */ + + +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +/* This function prepares for each 'too-large memory allocation' test by + performing a small successful malloc/free and resetting errno prior to + the actual test. */ +static void +test_setup (void) +{ + void *volatile ptr = malloc (16); + TEST_VERIFY_EXIT (ptr != NULL); + free (ptr); + errno = 0; +} + + +/* This function tests each of: + - malloc (SIZE) + - realloc (PTR_FOR_REALLOC, SIZE) + - for various values of NMEMB: + - calloc (NMEMB, SIZE/NMEMB) + - calloc (SIZE/NMEMB, NMEMB) + - reallocarray (PTR_FOR_REALLOC, NMEMB, SIZE/NMEMB) + - reallocarray (PTR_FOR_REALLOC, SIZE/NMEMB, NMEMB) + and precedes each of these tests with a small malloc/free before it. */ +static void +test_large_allocations (size_t size) +{ + void * ptr_to_realloc; + + test_setup (); + TEST_VERIFY (malloc (size) == NULL); + TEST_VERIFY (errno == ENOMEM); + + ptr_to_realloc = malloc (16); + TEST_VERIFY_EXIT (ptr_to_realloc != NULL); + test_setup (); + TEST_VERIFY (realloc (ptr_to_realloc, size) == NULL); + TEST_VERIFY (errno == ENOMEM); + free (ptr_to_realloc); + + for (size_t nmemb = 1; nmemb <= 8; nmemb *= 2) + if ((size % nmemb) == 0) + { + test_setup (); + TEST_VERIFY (calloc (nmemb, size / nmemb) == NULL); + TEST_VERIFY (errno == ENOMEM); + + test_setup (); + TEST_VERIFY (calloc (size / nmemb, nmemb) == NULL); + TEST_VERIFY (errno == ENOMEM); + + ptr_to_realloc = malloc (16); + TEST_VERIFY_EXIT (ptr_to_realloc != NULL); + test_setup (); + TEST_VERIFY (reallocarray (ptr_to_realloc, nmemb, size / nmemb) == NULL); + TEST_VERIFY (errno == ENOMEM); + free (ptr_to_realloc); + + ptr_to_realloc = malloc (16); + TEST_VERIFY_EXIT (ptr_to_realloc != NULL); + test_setup (); + TEST_VERIFY (reallocarray (ptr_to_realloc, size / nmemb, nmemb) == NULL); + TEST_VERIFY (errno == ENOMEM); + free (ptr_to_realloc); + } + else + break; +} + + +static long pagesize; + +/* This function tests the following aligned memory allocation functions + using several valid alignments and precedes each allocation test with a + small malloc/free before it: + memalign, posix_memalign, aligned_alloc, valloc, pvalloc. */ +static void +test_large_aligned_allocations (size_t size) +{ + /* PTR stores the result of posix_memalign but since all those calls + should fail, posix_memalign should never touch PTR. We set it to + NULL here and later on we check that it remains NULL after each + posix_memalign call. */ + void * ptr = NULL; + + size_t align; + + /* All aligned memory allocation functions expect an alignment that is a + power of 2. Given this, we test each of them with every valid + alignment from 1 thru PAGESIZE. */ + for (align = 1; align <= pagesize; align *= 2) + { + test_setup (); + TEST_VERIFY (memalign (align, size) == NULL); + TEST_VERIFY (errno == ENOMEM); + + /* posix_memalign expects an alignment that is a power of 2 *and* a + multiple of sizeof (void *). */ + if ((align % sizeof (void *)) == 0) + { + test_setup (); + TEST_VERIFY (posix_memalign (&ptr, align, size) == ENOMEM); + TEST_VERIFY (ptr == NULL); + } + + /* aligned_alloc expects a size that is a multiple of alignment. */ + if ((size % align) == 0) + { + test_setup (); + TEST_VERIFY (aligned_alloc (align, size) == NULL); + TEST_VERIFY (errno == ENOMEM); + } + } + + /* Both valloc and pvalloc return page-aligned memory. */ + + test_setup (); + TEST_VERIFY (valloc (size) == NULL); + TEST_VERIFY (errno == ENOMEM); + + test_setup (); + TEST_VERIFY (pvalloc (size) == NULL); + TEST_VERIFY (errno == ENOMEM); +} + + +#define FOURTEEN_ON_BITS ((1UL << 14) - 1) +#define FIFTY_ON_BITS ((1UL << 50) - 1) + + +static int +do_test (void) +{ + +#if __WORDSIZE >= 64 + + /* This test assumes that none of the supported targets have an address + bus wider than 50 bits, and that therefore allocations for sizes wider + than 50 bits will fail. Here, we ensure that the assumption continues + to be true in the future when we might have address buses wider than 50 + bits. */ + + struct rlimit alloc_size_limit + = { + .rlim_cur = FIFTY_ON_BITS, + .rlim_max = FIFTY_ON_BITS + }; + + setrlimit (RLIMIT_AS, &alloc_size_limit); + +#endif /* __WORDSIZE >= 64 */ + + DIAG_PUSH_NEEDS_COMMENT; +#if __GNUC_PREREQ (7, 0) + /* GCC 7 warns about too-large allocations; here we want to test + that they fail. */ + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif + + /* Aligned memory allocation functions need to be tested up to alignment + size equivalent to page size, which should be a power of 2. */ + pagesize = sysconf (_SC_PAGESIZE); + TEST_VERIFY_EXIT (powerof2 (pagesize)); + + /* Loop 1: Ensure that all allocations with SIZE close to SIZE_MAX, i.e. + in the range (SIZE_MAX - 2^14, SIZE_MAX], fail. + + We can expect that this range of allocation sizes will always lead to + an allocation failure on both 64 and 32 bit targets, because: + + 1. no currently supported 64-bit target has an address bus wider than + 50 bits -- and (2^64 - 2^14) is much wider than that; + + 2. on 32-bit targets, even though 2^32 is only 4 GB and potentially + addressable, glibc itself is more than 2^14 bytes in size, and + therefore once glibc is loaded, less than (2^32 - 2^14) bytes remain + available. */ + + for (size_t i = 0; i <= FOURTEEN_ON_BITS; i++) + { + test_large_allocations (SIZE_MAX - i); + test_large_aligned_allocations (SIZE_MAX - i); + } + +#if __WORDSIZE >= 64 + /* On 64-bit targets, we need to test a much wider range of too-large + sizes, so we test at intervals of (1 << 50) that allocation sizes + ranging from SIZE_MAX down to (1 << 50) fail: + The 14 MSBs are decremented starting from "all ON" going down to 1, + the 50 LSBs are "all ON" and then "all OFF" during every iteration. */ + for (size_t msbs = FOURTEEN_ON_BITS; msbs >= 1; msbs--) + { + size_t size = (msbs << 50) | FIFTY_ON_BITS; + test_large_allocations (size); + test_large_aligned_allocations (size); + + size = msbs << 50; + test_large_allocations (size); + test_large_aligned_allocations (size); + } +#endif /* __WORDSIZE >= 64 */ + + DIAG_POP_NEEDS_COMMENT; + + return 0; +} + + +#include