From patchwork Thu Nov 24 21:32:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 61091 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 0E21F3886C40 for ; Thu, 24 Nov 2022 21:33:56 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from bumble.birch.relay.mailchannels.net (bumble.birch.relay.mailchannels.net [23.83.209.25]) by sourceware.org (Postfix) with ESMTPS id 6FFD23852C57 for ; Thu, 24 Nov 2022 21:33:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6FFD23852C57 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 9184E140D9B; Thu, 24 Nov 2022 21:33:40 +0000 (UTC) Received: from pdx1-sub0-mail-a306.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id BA8F2140DCA; Thu, 24 Nov 2022 21:33:39 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1669325619; a=rsa-sha256; cv=none; b=EZgBAncZri/sTR/ZcknXpC94QVBzcpW+qrLMPcYw9isg05vyB9GTYMK1FpXu+8CrbBQT4n Ej8/U0AbKWjVcGM0YO6/7VBuMK83tpUq54aqj52zsqA1MP8ocmcN33VzkzUrqJwUxjRFdR 96g4r9++8YOifnt7JCt13Qj6nLj+AF+YDkVxdny0WuLEozIKuRtwPSAJmQIDDgg2CpdOXn xr6fQXsbwdlOvL1i/6BqI+M9zQrxi41QQALCf12XnPoLtJ1i1VQe2yV3sRZ1hcsN18myme 8SaM+2tkcxWM8AEOMxSmWPhWwTTtxqOqCPGsP2HUUJSKFc79Ci4ave9Em7ElSA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1669325619; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding:dkim-signature; bh=AezyItS7PcpZ9RFiuOIecrBctxgs9JcdtEyeuU0gSJE=; b=g9akctfumPSII5xlzjnKQUBeuvNbUnl78O/kyZs48WN8xp2haE6oh8IzSBtng9/VHyFGWu YAScFIZKiKuMpcS7GVfM0z6F+2A7a8y1RSUZN5rpczc1T+cIcl/Hnr/E0G8mYKL2K/nB/Q Ma3BpoHrSplgSD6WWaF98hTFXS7ecahb7gbno6AAH6KfSHmLaPB2w9le6RJVf0vPrmWsye lYTACxQYMff0NwGteF7xV0gKvdpA+l59pNmBV6M7/qn3TCOBkeduZl1vQHEnwCp4dWOi8H pNFIWKWeotYNDgONJ4G0M/LpH74BwtuG8UKOBjBsYVqTiqMM3LWj4SLbRzbdNg== ARC-Authentication-Results: i=1; rspamd-6dfdf8d5bc-ll976; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Average-Skirt: 580e2a8376c3d8d2_1669325620006_10927986 X-MC-Loop-Signature: 1669325620006:4251866451 X-MC-Ingress-Time: 1669325620006 Received: from pdx1-sub0-mail-a306.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.120.227.168 (trex/6.7.1); Thu, 24 Nov 2022 21:33:40 +0000 Received: from fedora.redhat.com (bras-base-toroon4834w-grc-23-76-68-24-147.dsl.bell.ca [76.68.24.147]) (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) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a306.dreamhost.com (Postfix) with ESMTPSA id 4NJB7l1L5Yz6t; Thu, 24 Nov 2022 13:33:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1669325619; bh=AezyItS7PcpZ9RFiuOIecrBctxgs9JcdtEyeuU0gSJE=; h=From:To:Cc:Subject:Date:Content-Transfer-Encoding; b=fIUuKFVtOZg9sCKA+E26JKwNLGUCp+jdbqICBM3dcRFbKv3DHadKHzqerKRFy3Mr9 I3WwURDKOXwA92P7bsG8StDI1Mu/Ch+sZg2Lyzot8Qu+BWVLeBy+RTJahsB7J1sSXb +zLtYL6Wz6grDmtNsh+N/BR1pxanqz92Mdu2hkt7W2sv4EJNB6zTrSTVSFylej5gmh gCRYgSzCgLclOtuHGV1vhHc33p2sKei46XFHNC30I6osNKMsdOpFQ0dct/bKjC8VMJ gdRXTFLoiaLBIycag1O3z13JhiZt9mcdN+XaCRvWnJodFLanIbYuTXtRqmt69JMHWP d3ZaCmKNu+nmg== From: Siddhesh Poyarekar To: libc-alpha@sourceware.org Cc: fweimer@redhat.com, carlos@redhat.com Subject: [RFC] Supporting malloc_usable_size Date: Thu, 24 Nov 2022 16:32:58 -0500 Message-Id: <20221124213258.305192-1-siddhesh@gotplt.org> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 X-Spam-Status: No, score=-3036.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: , Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" Hello, This is in context of this systemd issue: https://github.com/systemd/systemd/issues/22801 through which I had discovered that systemd was (ab)using malloc_usable_size to use spare space in an allocated object. This was discovered when _FORTIFY_SOURCE=3 flagged this as a buffer overflow, since the compiler is unable to see that the space beyond the allocation was safe to use. At that time, I thought this use was to avoid reallocating[1] when there's enough space in the usable size and that is not entirely false. I had then proposed during my Cauldron talk (and in discussions with some of you) that maybe providing a fast path for cases where the new size was within the current chunk size was the solution to the problem systemd was trying to solve with malloc_usable_size. However I was wrong, it looks like the malloc_usable_size usage goes far beyond simply optimizing allocation, it is used as a way to query sizes of objects and hence eliminating the need to pass their corresponding sizes. This is probably suboptimal (given that there's a call into libc.so at every point instead of what could have been just an additional parameter alongside the object) but it's the design pattern they've chosen. I have an idea to support this abuse and at the same time, satisfy the compiler by giving it a hint of the new size. This could be done within systemd by calling a dummy allocator (e.g. expand_to_usable) with the result of malloc_usable_size(), thus telling the compiler that the object has been expanded to the usable size, thus making accesses into the spare space well defined. I have a patch too (see below) to systemd that demonstrates how this could be achieved, it's been tested with gcc as well as clang and with _FORTIFY_SOURCE=3. The larger consequence of this patch though is that we further support the usage of malloc_usable_size for cases beyond diagnostics. Do we want to do that? If we do, should we also then make clear what kind of usage we support as a library, say, in the manual? We don't have a manual blurb for malloc_usable_size, so maybe this is a good opportunity to do that. Any other thoughts on malloc_usable_size? Thanks, Sid [1] https://developers.redhat.com/articles/2022/09/17/gccs-new-fortification-level#the_gains_of_improved_security_coverage_outweigh_the_cost --- src/basic/alloc-util.c | 5 +++++ src/basic/alloc-util.h | 16 ++++++++++++++-- src/test/test-alloc-util.c | 5 +++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/basic/alloc-util.c b/src/basic/alloc-util.c index b030f454b2..58cf7c7433 100644 --- a/src/basic/alloc-util.c +++ b/src/basic/alloc-util.c @@ -102,3 +102,8 @@ void* greedy_realloc0( return q; } + +void *expand_to_usable (void *ptr, size_t newsize _unused_) +{ + return ptr; +} diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index b38db7d473..0c0b4851f5 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "macro.h" @@ -184,6 +185,18 @@ void* greedy_realloc0(void **p, size_t need, size_t size); # define msan_unpoison(r, s) #endif +void *expand_to_usable (void *ptr, size_t newsize) _alloc_ (2); + +static inline size_t +_malloc_usable_size(void **ptr) +{ + if (*ptr == NULL) + return 0; + size_t full_size = malloc_usable_size (*ptr); + *ptr = expand_to_usable (*ptr, full_size); + return full_size; +} + /* This returns the number of usable bytes in a malloc()ed region as per malloc_usable_size(), in a way that * is compatible with _FORTIFY_SOURCES. If _FORTIFY_SOURCES is used many memory operations will take the * object size as returned by __builtin_object_size() into account. Hence, let's return the smaller size of @@ -193,8 +206,7 @@ void* greedy_realloc0(void **p, size_t need, size_t size); * too. Moreover, when NULL is passed malloc_usable_size() is documented to return zero, and * __builtin_object_size() returns SIZE_MAX too, hence we also return a sensible value of 0 in this corner * case. */ -#define MALLOC_SIZEOF_SAFE(x) \ - MIN(malloc_usable_size(x), __builtin_object_size(x, 0)) +#define MALLOC_SIZEOF_SAFE(x) _malloc_usable_size((void **) &(x)) /* Inspired by ELEMENTSOF() but operates on malloc()'ed memory areas: typesafely returns the number of items * that fit into the specified memory block */ diff --git a/src/test/test-alloc-util.c b/src/test/test-alloc-util.c index df6139005f..f9fd192576 100644 --- a/src/test/test-alloc-util.c +++ b/src/test/test-alloc-util.c @@ -170,8 +170,9 @@ TEST(malloc_size_safe) { size_t n = 4711; /* Let's check the macros and built-ins work on NULL and return the expected values */ - assert_se(MALLOC_ELEMENTSOF((float*) NULL) == 0); - assert_se(MALLOC_SIZEOF_SAFE((float*) NULL) == 0); + float *nullptr = NULL; + assert_se(MALLOC_ELEMENTSOF(nullptr) == 0); + assert_se(MALLOC_SIZEOF_SAFE(nullptr) == 0); assert_se(malloc_usable_size(NULL) == 0); /* as per man page, this is safe and defined */ assert_se(__builtin_object_size(NULL, 0) == SIZE_MAX); /* as per docs SIZE_MAX is returned for pointers where the size isn't known */