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-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-3.1 required=3.0 tests=AWL,BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,RCVD_IN_BL_SPAMCOP_NET,RCVD_IN_DNSWL_MED,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=no 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 4A19F1F8C6 for ; Wed, 14 Jul 2021 17:33:28 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9B9C139874CF for ; Wed, 14 Jul 2021 17:33:26 +0000 (GMT) Received: from bird.elm.relay.mailchannels.net (bird.elm.relay.mailchannels.net [23.83.212.17]) by sourceware.org (Postfix) with ESMTPS id BE4573987019; Wed, 14 Jul 2021 17:33:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BE4573987019 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org 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 B9574701D0B; Wed, 14 Jul 2021 17:33:10 +0000 (UTC) Received: from pdx1-sub0-mail-a5.g.dreamhost.com (100-101-162-69.trex.outbound.svc.cluster.local [100.101.162.69]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 4CF40701CCF; Wed, 14 Jul 2021 17:33:04 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a5.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384) by 100.101.162.69 (trex/6.3.3); Wed, 14 Jul 2021 17:33:10 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Lettuce-Duck: 74fa2cbb111d719f_1626283984736_1912507441 X-MC-Loop-Signature: 1626283984736:249029369 X-MC-Ingress-Time: 1626283984736 Received: from pdx1-sub0-mail-a5.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a5.g.dreamhost.com (Postfix) with ESMTP id B66EB8AC85; Wed, 14 Jul 2021 10:33:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gotplt.org; h=subject:to :cc:references:from:message-id:date:mime-version:in-reply-to :content-type:content-transfer-encoding; s=gotplt.org; bh=Mv7rN5 Or2ldr1t2N65HN9D780OQ=; b=NccNK9ORQDK2lOIU5ebMPW1bPBPRf5DVLRYtqM zUMyU5rHhvEw5iwoMJSjHAJHi1qqVkzy7MCrZabW1CsAv2jNZLsSNIG5TaiwLMWl C8OxvxxX2SaPf9OlWtUmHnvW7YniUE9Lch8snKfTDs1w60Fq+ShT/MfFsgI4kS9Y /CfN8= Received: from [192.168.1.139] (unknown [1.186.101.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a5.g.dreamhost.com (Postfix) with ESMTPSA id E7DA88AC84; Wed, 14 Jul 2021 10:33:00 -0700 (PDT) Subject: Re: [PATCH v8 03/10] Remove __morecore and __default_morecore To: Guillaume Morin References: <20210713073845.504356-1-siddhesh@sourceware.org> <20210713073845.504356-4-siddhesh@sourceware.org> <9925362a-d278-a0ad-f504-ae08ca93f628@gotplt.org> <20210714125415.GA24678@bender.morinfr.org> <45ba79b4-02e6-fae5-ae9b-db6c8a01aecb@gotplt.org> <20210714164204.GA1103@bender.morinfr.org> X-DH-BACKEND: pdx1-sub0-mail-a5 From: Siddhesh Poyarekar Message-ID: <6364b66d-14a8-3086-00ad-a0def6995c54@gotplt.org> Date: Wed, 14 Jul 2021 23:02:54 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20210714164204.GA1103@bender.morinfr.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US 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: , Cc: fweimer@redhat.com, Siddhesh Poyarekar , libc-alpha@sourceware.org Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 7/14/21 10:12 PM, Guillaume Morin wrote: > On 14 Jul 19:43, Siddhesh Poyarekar wrote: > Probably. But you surely know that most people get glibc from their > distributions and do not build their own glibc. My main box is running > Debian stable for example. The next release (bullseye) seems like it'll > be shipping 2.31 (where it's not deprecated yet, I believe). RHEL8 ships > with a libhugetlbfs rpm (so users do not need to build it at all unlike > Debian), which will become useless with this change! Most users will > never have seen the deprecation warning before they see the actual > removal. But that means it won't be useless in RHEL8 or current stable Debian/Ubuntu, so there's still time for libhugetlbfs to adapt to the change. > That said, I was personally aware since it was pointed out on a > libhugetlbfs github issue that it was deprecated in 2.32. I think the > hope was it would be replaced by something usable, and users would not > be left with *no* solution (I was also quietly hoping Eric Munson, the > libhugetlbfs maintainer would be reaching out). But if you remove it, I > am not quite sure what you expect libhugetlbfs users to do, really. As is evident, I personally haven't followed libhugetlbfs development and have been carrying forward work to remove __morecore that started at least a year ago, if not more. That said though, the deprecation warning (especially the fact that the release notes specifically mentioned malloc interpositioning as the alternative) ought to have made it clear that we weren't thinking of an alternative. > Well, I do not see why morecore() does not make sense as a plugin. I am > not seeing any technical argument supporting that it will prevent > clamping down the implementation provided you document the currently > acceptable semantics for this interface. Right now the debug DSO that this patch pulls out only supports the debugging features (malloc_check, mcheck, mtrace, old hooks) and they're not intended to be linked against, only used as compatibility for existing programs. If __morecore is to move there, we'd have to allow linking with the debug DSO, which we don't want to do because it multiplies the interfaces we need to support. We'd basically have two malloc interfaces to support then. > libhugetlbfs users are aware this only works in the main arena. I > certainly agree this is not ideal and this is definitely something that > could be fixed as well. But again, keep in mind people have been using > this for 15+ years. > > Obviously, if you're arguing to extend the glibc hookability so it works > for all arenas, I am all ears, certainly would help out and would be a > day one user :-) Unfortunately not, I'd like the system malloc to be as less hookable as possible because the hookability is a security nightmare and it prevents a lot of optimizations and hardening. > Again, you seem to be ignoring the 15 years of people using > libhugetblfs. It's been tested extensively by users. I certainly agree > it should get testing in the glibc testsuite, but it's actually trivial > afaict: > > - morecore() in effect has very simple semantics. It takes a size and > returns a pointer to the beginning of the new area. The only thing > that differs here from sbrk() is the fact that a new area might not be > contiguous with the previous segment. But that's supported in the > malloc code, otherwise libhugetlbfs morecore() would never have worked > (that's the MORECORE_CONTIGUOUS handling in the malloc code). That's > really _all_ you need to test. And I would presume non contiguous > heaps are implemented not just for morecore() sake (not sure though). > If that's case, no actual specific testing needs to be done. > > - If you _only_ allow morecore interposition through a DSO (as I was > suggesting), you do _not_ even need to support non contiguous heaps > (at least for morecore()) since at that point it becomes guaranteed > that morecore() will always point to the same implementation. Then > there is _nothing_ to test at that point. Proper morecore() > implementations can semantically behave exactly like sbrk! > > - A morecore2() hook that takes the old top of the heap as well as the > size would also allow libhugetlbfs() to implement a perfectly > continuous heap and so would semantically behave like sbrk(). But it > does not look like it's being considered here :-) The direction we want to take is to not have to support all this in the interest of simplifying the implementation and make it easier to reason, audit and debug. > Honestly from my PoV, some usual feature was added a while ago. The rest > of the code was changed without paying attention to the feature (bug > report ignored for 5 years as well). And you're using this as an > argument to remove it.... I don't think this is tech debt at all, the > feature is simply paying the price of being ignored. It does _seem_ to > me that little effort have been made to see if it was workable at all. Different developers have been involved in this and I suppose the developers who wrote the original implementation may even agree with you. However, technology has changed a lot over the years and many of the things that were thought to be useful are now either redundant, unwieldy or downright dangerous. The hooks fall somewhere between the last two because in the general case they're quite hard to reason and in the simplest case, they're unprotected indirections that have historically been used as vulnerability primitives for years. > To be clear, I am not the libhugetblfs maintainer, just a user (that > investigated the trimming issue). I think everybody is aware they can > fork the glibc malloc implementation. Surely you realize that it's not > ideal but even feasible for most users. I understand, I'm sorry it has come to this. I hope libhugetlbfs maintainers can find a way out sooner. Siddhesh