From patchwork Mon Nov 6 16:32:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arjun Shankar X-Patchwork-Id: 834847 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-86817-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="VRdCJBtw"; 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 3yVyly5HK8z9s7M for ; Tue, 7 Nov 2017 03:33:02 +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=MjoKax+TQKvDKWcHkXI4PCkLd7yjP JM5k8qk2vQUp5qn4lpPBnt7xwmuGnLDro7M1pOhNuqRhFbH0X20hpbvJVDlELm9T Z8HFxeGo4menaK1yqzthoFQQIzq9ApmQ9JP2PCtTFjEK3AerERTvDiAmWe+9yKh2 ijAA89jxF/3p7g= 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=3MES4qFbpOdS39d4+ykhgzNvaDo=; b=VRd CJBtwSsg9ZQU2+Dc0Ay5AkmO61HDtiUulQgbD015WuwGF7E0x3RBnqELxw2oYQry OXaP9lhEkh7LWup3OjyazNwPMAZ5WtrJ0KaR+bCeQm1eiM8MRL/hdLBEzyaDR5hM cbaoafHqoyYyKeBQGQVyF68ZlMVfErs0Wq9NYb/w= Received: (qmail 125441 invoked by alias); 6 Nov 2017 16:32:54 -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 125375 invoked by uid 89); 6 Nov 2017 16:32:53 -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, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=ranging X-HELO: aloka.lostca.se Date: Mon, 6 Nov 2017 16:32:48 +0000 From: Arjun Shankar To: libc-alpha@sourceware.org Subject: [PATCH] Fix integer overflow in malloc when tcache is enabled [BZ #22375] Message-ID: <20171106163248.GA48861@aloka.lostca.se> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.7.1 (2016-10-04) When the per-thread cache is enabled, __libc_malloc uses request2size (which does not perform an overflow check) to calculate the chunk size from the requested allocation size. This leads to an integer overflow causing malloc to incorrectly return the last successfully allocated block when called with a very large size argument (close to SIZE_MAX) instead of returning NULL and setting errno to ENOMEM. This patch uses checked_request2size instead, removing the overflow, and adds a new test to guard against regressions. ChangeLog: 2017-11-06 Arjun Shankar [BZ #22375] * malloc/malloc.c (__libc_malloc): Use checked_request2size instead of request2size. * malloc/tst-malloc-too-large.c: New test. * malloc/Makefile: Add tst-malloc-too-large. --- Tested with no regressions on x86_64, ppc64, ppc64le, and x86 (but the new test is unsupported on 32 bit targets). malloc/Makefile | 1 + malloc/malloc.c | 3 +- malloc/tst-malloc-too-large.c | 139 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 malloc/tst-malloc-too-large.c diff --git a/malloc/Makefile b/malloc/Makefile index 7ae3d82..a01279d 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -35,6 +35,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-interpose-thread \ tst-alloc_buffer \ tst-malloc-tcache-leak \ + tst-malloc-too-large \ tests-static := \ tst-interpose-static-nothread \ diff --git a/malloc/malloc.c b/malloc/malloc.c index f94d51c..2ebd97f 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -3022,7 +3022,8 @@ __libc_malloc (size_t bytes) return (*hook)(bytes, RETURN_ADDRESS (0)); #if USE_TCACHE /* int_free also calls request2size, be careful to not pad twice. */ - size_t tbytes = request2size (bytes); + size_t tbytes; + checked_request2size (bytes, tbytes); size_t tc_idx = csize2tidx (tbytes); MAYBE_INIT_TCACHE (); diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c new file mode 100644 index 0000000..002a893 --- /dev/null +++ b/malloc/tst-malloc-too-large.c @@ -0,0 +1,139 @@ +/* Test and verify that too-large memory allocations fail with ENOMEM. + Copyright (C) 2017 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 + . */ + + +#include + + +#if __WORDSIZE < 64 + + +static int +do_test (void) +{ + FAIL_UNSUPPORTED ("Test does not support targets with word size < 64-bit"); +} + + +#else /* __WORDSIZE >= 64 */ + + +#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. The small malloc/free is done to catch regressions + such as the one reported here: + https://sourceware.org/bugzilla/show_bug.cgi?id=22375 */ +static void +test_setup (void) +{ + void * ptr = malloc (16); + TEST_VERIFY_EXIT (ptr != NULL); + free (ptr); + errno = 0; +} + + +/* This function tests each of: + - malloc (SIZE) + - calloc (1, SIZE) + - calloc (SIZE, 1) + - realloc (PTR_FOR_REALLOC, SIZE) + and precedes each of these tests with a small malloc 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); + + test_setup (); + TEST_VERIFY (calloc (size, 1) == NULL); + TEST_VERIFY (errno == ENOMEM); + + test_setup (); + TEST_VERIFY (calloc (1, 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); +} + + +#define FOURTEEN_ON_BITS ((1UL << 14) - 1) +#define FIFTY_ON_BITS ((1UL << 50) - 1) + + +static int +do_test (void) +{ + size_t lsbs, msbs, size; + + 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 + + /* The allocation SIZE used during each iteration is obtained by combining + LSBS and MSBS. In order to catch all interesting cases but avoid + looping through every too-large size, i.e. [2^50, 2^64), we will have + two loops with different iteration patterns for LSBs and MSBs. */ + + /* Loop 1: Ensure that all allocations with SIZE close to SIZE_MAX fail: + The 14 MSBs are "all ON" during every iteration, + the 50 LSBs are decremented starting from "all ON", for 1<<14 + iterations. */ + for (lsbs = FIFTY_ON_BITS; lsbs > FIFTY_ON_BITS - (1UL << 14); lsbs--) + { + size = (FOURTEEN_ON_BITS << 50) | lsbs; + test_large_allocations (size); + } + + /* Loop 2: 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 (msbs = FOURTEEN_ON_BITS; msbs >= 1; msbs--) + { + size = (msbs << 50) | FIFTY_ON_BITS; + test_large_allocations (size); + + size = msbs << 50; + test_large_allocations (size); + } + + DIAG_POP_NEEDS_COMMENT; + + return 0; +} + + +#endif /* __WORDSIZE >= 64 */ + + +#include