From patchwork Wed Jul 7 17:34:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 44207 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8225E395C82A for ; Wed, 7 Jul 2021 17:35:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8225E395C82A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1625679312; bh=GJ0iB3Mj9JfQ82gW/4ThAiU7/dsnGixKpw9bUvlS58U=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ynTf08EQpHO5u2DBJDAfeUJiHJDc91PVSogDdF5Im0l0I24SW4hI8HePtqf/69ToD Nkp3c7qwua6wfRDkQnBuqq8wk0EHKXJazofGFEF5j6GT8usluNmB8/ald3VxrGNfsd MgeCiALSUvRR/usMJfJPrw9ixli2sLcaZqa47meU= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from bumble.maple.relay.mailchannels.net (bumble.maple.relay.mailchannels.net [23.83.214.25]) by sourceware.org (Postfix) with ESMTPS id BC3B1384841C for ; Wed, 7 Jul 2021 17:34:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BC3B1384841C X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 13F22702C84; Wed, 7 Jul 2021 17:34:47 +0000 (UTC) Received: from pdx1-sub0-mail-a24.g.dreamhost.com (100-96-27-225.trex.outbound.svc.cluster.local [100.96.27.225]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id F03A8702817; Wed, 7 Jul 2021 17:34:45 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a24.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384) by 100.96.27.225 (trex/6.3.3); Wed, 07 Jul 2021 17:34:47 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Whimsical-Abaft: 18c57df36c59816a_1625679286726_1932019471 X-MC-Loop-Signature: 1625679286726:3665938801 X-MC-Ingress-Time: 1625679286726 Received: from pdx1-sub0-mail-a24.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a24.g.dreamhost.com (Postfix) with ESMTP id AC9B38B5AE; Wed, 7 Jul 2021 10:34:45 -0700 (PDT) Received: from rhbox.intra.reserved-bit.com (unknown [1.186.101.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a24.g.dreamhost.com (Postfix) with ESMTPSA id E7C8D8AD25; Wed, 7 Jul 2021 10:34:42 -0700 (PDT) X-DH-BACKEND: pdx1-sub0-mail-a24 To: libc-alpha@sourceware.org Subject: [PATCH v2] Harden tcache double-free check Date: Wed, 7 Jul 2021 23:04:26 +0530 Message-Id: <20210707173426.4001976-1-siddhesh@sourceware.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <87a6my2gdc.fsf@oldenburg.str.redhat.com> References: <87a6my2gdc.fsf@oldenburg.str.redhat.com> MIME-Version: 1.0 X-Spam-Status: No, score=-3494.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Siddhesh Poyarekar via Libc-alpha From: Siddhesh Poyarekar Reply-To: Siddhesh Poyarekar Cc: fweimer@redhat.com, Eyal Itkin Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" The tcache allocator layer uses the tcache pointer as a key to identify a block that may be freed twice. Since this is in the application data area, an attacker exploiting a use-after-free could potentially get access to the entire tcache structure through this key. A detailed write-up was provided by Awarau here: https://awaraucom.wordpress.com/2020/07/19/house-of-io-remastered/ Replace this static pointer use for key checking with one that is generated at malloc initialization. The first attempt is through getrandom with a fallback to random_bits(), which is a simple pseudo-random number generator based on the clock. The fallback ought to be sufficient since the goal of the randomness is only to make the key arbitrary enough that it is very unlikely to collide with user data. Co-authored-by: Eyal Itkin --- Change from v1: - Expanded comment about the double free check. malloc/arena.c | 8 ++++++++ malloc/malloc.c | 37 +++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/malloc/arena.c b/malloc/arena.c index 7eb110445e..991fc21a7e 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -287,6 +287,10 @@ extern struct dl_open_hook *_dl_open_hook; libc_hidden_proto (_dl_open_hook); #endif +#if USE_TCACHE +static void tcache_key_initialize (void); +#endif + static void ptmalloc_init (void) { @@ -295,6 +299,10 @@ ptmalloc_init (void) __malloc_initialized = 0; +#if USE_TCACHE + tcache_key_initialize (); +#endif + #ifdef USE_MTAG if ((TUNABLE_GET_FULL (glibc, mem, tagging, int32_t, NULL) & 1) != 0) { diff --git a/malloc/malloc.c b/malloc/malloc.c index bb9a1642aa..a3525f71da 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -252,6 +252,10 @@ #include +/* For tcache double-free check. */ +#include +#include + /* Debugging: @@ -3091,7 +3095,7 @@ typedef struct tcache_entry { struct tcache_entry *next; /* This field exists to detect double frees. */ - struct tcache_perthread_struct *key; + uintptr_t key; } tcache_entry; /* There is one of these for each thread, which contains the @@ -3108,6 +3112,31 @@ typedef struct tcache_perthread_struct static __thread bool tcache_shutting_down = false; static __thread tcache_perthread_struct *tcache = NULL; +/* Process-wide key to try and catch a double-free in the same thread. */ +static uintptr_t tcache_key; + +/* The value of tcache_key does not really have to be a cryptographically + secure random number. It only needs to be arbitrary enough so that it does + not collide with values present in applications. If a collision does happen + consistently enough, it could cause a degradation in performance since the + entire list is checked to check if the block indeed has been freed the + second time. The odds of this happening are exceedingly low though, about 1 + in 2^wordsize. There is probably a higher chance of the performance + degradation being due to a double free where the first free happened in a + different thread; that's a case this check does not cover. */ +static void +tcache_key_initialize (void) +{ + if (__getrandom (&tcache_key, sizeof(tcache_key), GRND_NONBLOCK) + != sizeof (tcache_key)) + { + tcache_key = random_bits (); +#if __WORDSIZE == 64 + tcache_key = (tcache_key << 32) | random_bits (); +#endif + } +} + /* Caller must ensure that we know tc_idx is valid and there's room for more chunks. */ static __always_inline void @@ -3117,7 +3146,7 @@ tcache_put (mchunkptr chunk, size_t tc_idx) /* Mark this chunk as "in the tcache" so the test in _int_free will detect a double free. */ - e->key = tcache; + e->key = tcache_key; e->next = PROTECT_PTR (&e->next, tcache->entries[tc_idx]); tcache->entries[tc_idx] = e; @@ -3134,7 +3163,7 @@ tcache_get (size_t tc_idx) malloc_printerr ("malloc(): unaligned tcache chunk detected"); tcache->entries[tc_idx] = REVEAL_PTR (e->next); --(tcache->counts[tc_idx]); - e->key = NULL; + e->key = 0; return (void *) e; } @@ -4437,7 +4466,7 @@ _int_free (mstate av, mchunkptr p, int have_lock) trust it (it also matches random payload data at a 1 in 2^ chance), so verify it's not an unlikely coincidence before aborting. */ - if (__glibc_unlikely (e->key == tcache)) + if (__glibc_unlikely (e->key == tcache_key)) { tcache_entry *tmp; size_t cnt = 0;