From patchwork Mon Apr 15 14:32:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wilco Dijkstra X-Patchwork-Id: 32292 Received: (qmail 108539 invoked by alias); 15 Apr 2019 14:33:04 -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 108526 invoked by uid 89); 15 Apr 2019 14:33:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=H*Ad:U*dj, sk:memory_ X-HELO: EUR02-AM5-obe.outbound.protection.outlook.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=3fWt8zk8cgXr6l2dTZrHchDbgQZ80xe+tz+kshzRwZs=; b=A8mS38cgVotnx+hpP9CY4F9QpWQZg0gd/3MYgG1kEuTTDnT9shWBfanXpjKadrGAMjCZ6jDg1iWzYn4rZGZ4xeigEI14OPfKkqymKRWcFSaXq8od7vTjn2n0+ozxk+BlHkCCWUbljjHZ0F0zMwmbZYkuDcVT18gw3w9hec+T2nE= From: Wilco Dijkstra To: "libc-alpha@sourceware.org" CC: nd , DJ Delorie Subject: [PATCH] Fix tcache count maximum Date: Mon, 15 Apr 2019 14:32:59 +0000 Message-ID: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Wilco.Dijkstra@arm.com; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED There are 2 issues with tcache count: the tcache counts[] array is a char, which may be signed and has a very small range and thus may overflow. When setting it, there is no overflow check. Eg. export GLIBC_TUNABLES=glibc.malloc.tcache_count=4096 leads to various crashes in benchtests: Running /build/glibc/benchtests/bench-strcoll bench-strcoll: malloc.c:2949: tcache_get: Assertion `tcache->counts[tc_idx] > 0' failed. Aborted Btw is this kind of crash regarded as a security risk? It's easy to do a denial of service this way if you're able to set an environment variable. ChangeLog: 2019-04-15 Wilco Dijkstra * malloc/malloc.c (tcache_perthread_struct): Use an unsigned short for counts array. (MAX_TCACHE_COUNT): New define. (do_set_tcache_count): Only update if count is small enough. diff --git a/malloc/malloc.c b/malloc/malloc.c index 801ba1f499b566e677b763fc84f8ba86f4f7ccd0..4db7283cc27118cd7d39410febf7be8f7633780a 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -2915,10 +2915,12 @@ typedef struct tcache_entry time), this is for performance reasons. */ typedef struct tcache_perthread_struct { - char counts[TCACHE_MAX_BINS]; + unsigned short counts[TCACHE_MAX_BINS]; tcache_entry *entries[TCACHE_MAX_BINS]; } tcache_perthread_struct; +#define MAX_TCACHE_COUNT 65535 /* Maximum value of counts[] entries. */ + static __thread bool tcache_shutting_down = false; static __thread tcache_perthread_struct *tcache = NULL; @@ -5114,8 +5116,11 @@ do_set_tcache_max (size_t value) static __always_inline int do_set_tcache_count (size_t value) { - LIBC_PROBE (memory_tunable_tcache_count, 2, value, mp_.tcache_count); - mp_.tcache_count = value; + if (value <= MAX_TCACHE_COUNT) + { + LIBC_PROBE (memory_tunable_tcache_count, 2, value, mp_.tcache_count); + mp_.tcache_count = value; + } return 1; }