From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 8103C1F531 for ; Mon, 10 Aug 2020 13:36:09 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 457F4385DC1A; Mon, 10 Aug 2020 13:36:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 457F4385DC1A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1597066568; bh=Pe8FpzR0bLr+vDZ2H3C9xzlbpmN6JOMb3vDJCceuHlc=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=u57DflVBJGTJf+wCQMDMgsgXDEqWYQ+Ur7HT5cgjPev0wPGYDehuMZWxLDgP8jWC+ OQAs0Lah+vPiyi84eBBcad/F0pm6A8Cl6CeF9mOOCl7rtE0LKX3GMI6XUNPpZWQQCN DSjVak8Wxg2KOKc/4UCvYKhiEawvDOsILM4fBfD8= Received: from mail-il1-x143.google.com (mail-il1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) by sourceware.org (Postfix) with ESMTPS id 9F87E3857C53 for ; Mon, 10 Aug 2020 13:36:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9F87E3857C53 Received: by mail-il1-x143.google.com with SMTP id x1so7519079ilp.7 for ; Mon, 10 Aug 2020 06:36:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Pe8FpzR0bLr+vDZ2H3C9xzlbpmN6JOMb3vDJCceuHlc=; b=BNpgd7fOuOICFd6jIOhhvpq7cv6HmYzKRC2A0w+P66W/8HkiygGkU6udRFWaVQbRH2 yBCRoVDpIYX+/NcX8AQgtNm9kYblTLw2XVyZTNSKPB/9Vixw2FEOs5G04Zo+XLIhSJhR Mmz3LgwAkVmRHS3LDgYSMg4vF4ckfkO4HufQusihoAtm2d82f9G6x0i/v3z5R3fTroGw S6mBuBw+M0B1CXinaeSHTqj5qYMN516miv0lFvWXv3sCAlgGSYhFWwgzT82jpXsqkizs Hf/WlmhY/OZLJ7Ju799QEKAyxkQ52mLf74G6UKYFXW3hLQVApHFSrWazqGUMCUcvwugF 6viA== X-Gm-Message-State: AOAM532jPrwbBmQ7gPRFpcime79+WAnY60R8xs/wEqJW0+4rGcMBA+v6 +bcV6jFOWzF72HsT5iMfGU9wWYYeQswr9K/1NmM= X-Google-Smtp-Source: ABdhPJzl6V9alwHL+srCUiw9nFvFzbh1Zd3oAaMQOQ1kZ8tcmU8Tc8kyp0ckipUcv9HhYKoH24iaePAkQX7WzAHB5GU= X-Received: by 2002:a05:6e02:1107:: with SMTP id u7mr8695482ilk.165.1597066564892; Mon, 10 Aug 2020 06:36:04 -0700 (PDT) MIME-Version: 1.0 References: <22dd78e7-13a7-0bec-37ba-f3e32a9630ab@redhat.com> <5c6aa5c1-7962-6f05-a114-0d79c9cd9bc5@redhat.com> In-Reply-To: <5c6aa5c1-7962-6f05-a114-0d79c9cd9bc5@redhat.com> Date: Mon, 10 Aug 2020 16:35:53 +0300 Message-ID: Subject: Re: [PATCH] Update tcache double-free check To: "Carlos O'Donell" Content-Type: text/plain; charset="UTF-8" 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: , From: Eyal Itkin via Libc-alpha Reply-To: Eyal Itkin Cc: Florian Weimer , GNU C Library Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" The design of this feature started by stating that "any arbitrary value" will suffice. Which will indeed be enough when protecting against a double free vulnerability assuming the attacker has no control over the content of the memory that is going to be free()ed twice. However, if we look at a proper threat model, we have the following: 1. Attacker that can't control the content of the memory to be free()ed 2. Attacker that has full/partial control over the content of the memory to be free()ed When examining the double free check, we can see that a double free operation will be caught if the EXACT key will match. Which basically means that if the attacker can modify even a single bit in the stored key, the check will be bypassed. Even, without the attacker knowing what was the original key. This basically means that the only advantage of using a random value is to avoid using a constant arbitrary value that might collide (repeatedly) with specific values of the program, and will demand slower checks in the tcache so as to rule out the possibility of a double free. In that sense, double-free across threads will never be caught, as the thread will still check it's own tcache struct and will never find the other thread's buffer inside their own struct. Usually, random values make a defense stronger because the attacker has to guess each and every one of the bits. However, after performing the analysis, it seems like we have the opposite case here. If the attacker can modify even a single bit, a double-free operation will be possible, as the check will be bypassed. Due to that, I recommend using a random value per process according to the original scheme of getrandom() and a backup of "~tcache". Keeping in mind that the ASLR bits will supply some random, and that ~tcache will never be a value we expect to see in memory and won't ever be a mapped memory address that could be used by a use-after-free vulnerability as the original key ("tcache") was exploited. I don't see any value in a per-thread random key, as again, the random value will only help us avoid collisions with values from the program's memory. The overall scheme for accumulating random between threads might be useful in the future, for a cookie-style security check, however I convinced myself that it won't be needed in this case. Once a decision will be made, please notify me, and I'll update the patch accordingly. On Mon, Aug 10, 2020 at 4:12 PM Carlos O'Donell wrote: > > On 8/10/20 9:07 AM, Eyal Itkin wrote: > > Updated the patch to perform an atomic update operation on the global > > entropy state, so to avoid races when multiple threads are initialized > > simultaneously. The patch now accumulates entropy between threads, > > while still using the backup case of ~tcache to take care of cases in > > which no entropy was yet to be available. > > > > As Carlos mentioned earlier, I guess you will want to discuss this > > patch before integrating it. Also, feel free to update the patch if > > needed in case I missed some whitespace / long line coding style. > > Thank you for putting this together. I need to spend some time thinking > more deeply on this and considering where the right balance might lie > between a per-process value and a per-thread value. Particularly with > respect to the tradeoff between maintaining the code and security. > > Do you have any strong opinions on the use of a per-thread vs. per-process > value? > > This patch is third on my queue. > > My queue is currently: > - NSS configuration reloading (DJ Delorie) > - DSO sorting DFS (Chung-Lin Tang) > - Tcache double-free check (Eyal Itkin) > > -- > Cheers, > Carlos. >