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.3 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (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 795871F5AE for ; Sat, 25 Jul 2020 21:07:41 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A6F943857C6B; Sat, 25 Jul 2020 21:07:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A6F943857C6B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1595711260; bh=z1HzD+DCHe4+cLBJHpTagxHNG5v5xK6S0/PU/5/di9o=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=sJE0stnXa6gbQcLuxHKWWgGS8/q5WVKBDLBxde4ifOXJ2RffRR93F1t44LDuIDQgD 2Q3BGl3iPNR7henIGa6YjMuF39BaImJBIeZkgBDtNTUJOIuiOcVfUHVcrDLfThK2BN OO5htmCxYwyapc5Gs+DVzKgg2AP+0qr6aI6fEprs= Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 1E9903857C58 for ; Sat, 25 Jul 2020 21:07:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1E9903857C58 Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-347-VLAxYCw9OZKa-0wybiMnZQ-1; Sat, 25 Jul 2020 17:07:36 -0400 X-MC-Unique: VLAxYCw9OZKa-0wybiMnZQ-1 Received: by mail-qt1-f199.google.com with SMTP id d2so5172547qtn.8 for ; Sat, 25 Jul 2020 14:07:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=z1HzD+DCHe4+cLBJHpTagxHNG5v5xK6S0/PU/5/di9o=; b=SHvDZm84fq/yOCs8lezECOiWQNVnDd3DvkomV5vPWi+AMbrE5VRvHphEV4xWByKYdb pogXkR7DkM2nIzpU6TAQj1kIA/Zc3HARFUlkI2RN91xipkOML+euqqRAPit4TXC2nG9s UhCGBeG2zxA6wjxK/gJ8SVOFznjEI9C9/AS81CCOxcNRVge5G6X/EihDUsV7jasy56wx bXCpRlWvzAwMmOznzzCeiPml9Wyd2nsoW4IXYLolw3suSBBDhSdPemxDsjF4fK3ZfwwU 18dxS1frH/7XsGyJInao1/4I5nmSJwkBdf+y3WrbE1hvqKWy2htViFYDzClgKeYOFunu onow== X-Gm-Message-State: AOAM530sV0Zb0EZGEkon7HjlcWeU/2yY+jYpoh/iFGhuk37f3HtGrT0h 178WyjrI79CbmndCTJzHvErcy5TZPS14+SZHzmHpe3fw/RhBLgafzHYF1crmFhltqP5jxdieRTc o7QPd8u3olFCGfdT+MxGj X-Received: by 2002:ac8:3042:: with SMTP id g2mr15663904qte.224.1595711255134; Sat, 25 Jul 2020 14:07:35 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwb64BOwij1TF540XrmVL8JeBW64d5gPZhZfaOXrbCPJbOLHd/PxXhL+6yD5/Z1g/l4iEDCHw== X-Received: by 2002:ac8:3042:: with SMTP id g2mr15663887qte.224.1595711254895; Sat, 25 Jul 2020 14:07:34 -0700 (PDT) Received: from [192.168.1.4] (198-84-170-103.cpe.teksavvy.com. [198.84.170.103]) by smtp.gmail.com with ESMTPSA id h124sm12604543qkd.84.2020.07.25.14.07.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 25 Jul 2020 14:07:33 -0700 (PDT) Subject: Re: [PATCH] Update tcache double-free check To: Eyal Itkin , Florian Weimer References: Organization: Red Hat Message-ID: <22dd78e7-13a7-0bec-37ba-f3e32a9630ab@redhat.com> Date: Sat, 25 Jul 2020 17:07:32 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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: Carlos O'Donell via Libc-alpha Reply-To: Carlos O'Donell Cc: GNU C Library Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On 7/25/20 6:39 AM, Eyal Itkin wrote: > I've gone over the compiled binaries before/after the patch, and the > change on my x64 binary is as follows: no change to tcache_get() or > tcache_put(), and an addition of 2 asm instructions to _int_free(). If > we would have only used ~tcache this would have been reduced to an > addition of a single asm instruction in _int_free(). > > The patch's impact on benchtests/bench-malloc-(simple,thread) is > negligible, and is less than the natural noise I get on my GCP > instance between two consecutive benchmark executions. > > Since the call to getrandom() is only on tcache_init(), hence once per > thread, I'm fine with it being called for all threads, but it is > really for you to decide. > > There is a possible fix to make sure that the entropy we get from > getrandom() will be accumulated, thus helping all future threads that > might need it. We won't be able to update the keys of previous threads > (if they lacked entropy), but we can enrich future threads with the > entropy even if they failed to use getrandom(). This solution will > also enable us to use all of the received entropy from getrandom() > even when we get less than the wanted amount of bytes. > > The solution for such an entropy accumulation is quite simple: > > static __thread size_t tcache_key = 0; > static size_t tcache_entropy = 0; > > void tcache_init() > { > ... > /* Attempt to get a random key for the double-free checks. */ > size_t local_entropy = 0; > getrandom (&local_entropy, sizeof(local_entropy), GRND_NONBLOCK); > /* always accumulate it with the past seen entropy */ > tcache_entropy ^= local_entropy; > /* always use the basic alternative of ~tcache in case we didn't > have any entropy. */ > tcache_key = tcache_entropy ^ ~((size_t) tcache); > } > > As can be seen, in case that all calls to getrandom() failed, and we > received nothing from it, we would still use ~tcache, just like > before. In addition, this use of "~tcache" also adds the entropy from > ASLR to the tcache_key, so we don't lose anything from using it. > > Please advise me of the best way to proceed. I like the accumulation of entropy. I'm a little worried about thread startup and shutdown costs, but I haven't measured that yet. I'm also busy getting the release out the door (I'm the release manager). If you have thread startup/shutdown costs that would be interesting for me to see as a reviewer. I'd like to see Florian's opinion on this. I fully expect that if you flesh this out you'll have to handle the getrandom failure case, and the atomics required to avoid the data race with multiple threads and global state. However, hold off on that until we get a big more consensus if we really want to go to this level of complexity. It would be good to keep the entropy we have in the event the system is under some kind of entropy starvation at a later point (I've seen at least one of these for TCP sequence numbers, read and drop from /dev/random etc.) -- Cheers, Carlos.