From patchwork Tue Dec 27 14:02:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 2952 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:c95:b0:82:e4b3:40a0 with SMTP id p21csp2650569dyk; Tue, 27 Dec 2022 06:03:52 -0800 (PST) X-Google-Smtp-Source: AMrXdXvjR6atFCZXEJYl1rrVcBTSvZzCkKFt1GmWNc9GKDI3Aae4JsyLO59U9BzG/7nBwAKCnL+A X-Received: by 2002:a4a:3311:0:b0:4a0:b7b9:f1f0 with SMTP id q17-20020a4a3311000000b004a0b7b9f1f0mr9228997ooq.1.1672149832677; Tue, 27 Dec 2022 06:03:52 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672149832; cv=none; d=google.com; s=arc-20160816; b=PjQGGUJX9NzH+lCKSQtyO6go8AfxLmkMews3gzktX8I727vcTB6Nys1As8cxkrqIUv 94xfeiIsucVxYQAZUp0gzhvqWfYSLUIJ/lIGyy83buj0p9/+Dm7fL15rUU0FNaQnU866 aD3mpJNEtORWnbOMJVEFIyvqweOHUqc0eUvWY4D+S3qnoPPA8ch0zVzbWK29l1OGXlC5 0wUhaOEMpldiwf4fbtDOwh0ZWdnBxzsqTbjpn+nU49gObKkApY2D0GioNIDFm/Pxs4Um ICb5ht9LzIUl4IWpoE3qMU311R7kRWKDnLraGAdrS7MEF6aUsxYZ2ITQCPE1LpLWuFXg KcKw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature; bh=sp6ppv4TyVPXsV/l6EBvb9XZ4+9wtG8s4tllmdODPzE=; b=qpf/OaStrsp8AMJkKHMLCAtHZ4f/AYzrq8sZHpM88fisQfgnOlqjylJVCQlZjL13Ut LxTLEdjhyBPhL2/xYacVXqdnysBrGBh09uCvDMxLC8Je/hq9na958kJVOwP5BKuRpdcP flQ0Dl8jn/Qg8wAA2YY9g8EW1waiYhUTF+PFXrLCnNmLSTxn8jPOL8o/pdkRsawssz/J CEEHXvw1oNwqwaVDjYwzV+IISb33vB+AWC8OZfQaRS/E1wOGlF8Fl3QRZB47rgDwN1OH uHHfC15jnr7NCadGUEtx7Pw0yQI98Lyi/f5RJ3bVH02KyS9bI8Q+7OQBVdlSDcUFMxpC N0Yg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=JjBK7wh5; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=hjjWYv+0; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id bb6-20020a056820160600b004808d11e074si8729709oob.46.2022.12.27.06.03.52 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 27 Dec 2022 06:03:52 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=JjBK7wh5; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=hjjWYv+0; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1pAAXj-0007l3-6v; Tue, 27 Dec 2022 14:02:59 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1pAAXh-0007kq-KI for openvpn-devel@lists.sourceforge.net; Tue, 27 Dec 2022 14:02:57 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=2S2nEow/LUyvgpDj70gY/NA0EfG9lIgOtfAInXl58cs=; b=JjBK7wh5Wf5Oq58XAIXPPCYHnb IJlGg3aTcFcNISfttrwvUbMfYD1v1MpRcCpIVcuA6yt7zMfI3HuEWoR9gIf6tdR2ARLw2W8zIPMMs 0FzeeCfZAVXhuU/qXpSkYaWQh2BE1nFxalgXsBA2vGK3CRf5FTfwxhc5cWzEKcZsBHrA=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=2S2nEow/LUyvgpDj70gY/NA0EfG9lIgOtfAInXl58cs=; b=hjjWYv+0dPEFo5hUU8yUuc7k8r hCJDsjzG2iWh94m7OkAqZh0mgp7Y7yFQuIN8mFmzT0bydRUMInL6w9s6Lhq/nLvgugFqNnZH7/zQR dapkScxOudtZLjOaLtq2U0jMlfuUDnZcwj5loRvHCOESOh4j+fGXHmLrkgf1sQUx7Nf0=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1pAAXg-00E5SE-LC for openvpn-devel@lists.sourceforge.net; Tue, 27 Dec 2022 14:02:57 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.95 (FreeBSD)) (envelope-from ) id 1pAAXZ-0004uI-6r for openvpn-devel@lists.sourceforge.net; Tue, 27 Dec 2022 15:02:49 +0100 Received: (nullmailer pid 3524992 invoked by uid 10006); Tue, 27 Dec 2022 14:02:49 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Tue, 27 Dec 2022 15:02:45 +0100 Message-Id: <20221227140249.3524943-2-arne@rfc2549.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221227140249.3524943-1-arne@rfc2549.org> References: <20221227140249.3524943-1-arne@rfc2549.org> MIME-Version: 1.0 X-Spam-Score: 0.3 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: The realloc logic has the problem that it relies on the memory being deallocated by uninit_options rather than by freeing the gc. This does not always happen in all code path. Especially the crypto se [...] Content analysis details: (0.3 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 SPF_NONE SPF: sender does not publish an SPF Record X-Headers-End: 1pAAXg-00E5SE-LC Subject: [Openvpn-devel] [PATCH 2/2] Replace realloc with new gc_realloc function X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1753376182668727988?= X-GMAIL-MSGID: =?utf-8?q?1753376182668727988?= The realloc logic has the problem that it relies on the memory being deallocated by uninit_options rather than by freeing the gc. This does not always happen in all code path. Especially the crypto selftest run by make check will not call uninit_options. This introduces a gc_realloc function that ensures that the pointer is instead freed when gc_free is called. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/buffer.c | 33 ++++++++++++++++++++++++++ src/openvpn/buffer.h | 13 ++++++++++ src/openvpn/options.c | 6 ++--- tests/unit_tests/openvpn/test_buffer.c | 32 +++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 575d45a1f..e3d5ba241 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -412,6 +412,39 @@ gc_malloc(size_t size, bool clear, struct gc_arena *a) return ret; } +void * +gc_realloc(void *ptr, size_t size, struct gc_arena *a) +{ + void *ret = realloc(ptr, size); + check_malloc_return(ret); + if (a) + { + if (ptr && ptr != ret) + { + /* find the old entry and modify it if realloc changed + * the pointer */ + struct gc_entry_special *e = NULL; + for (e = a->list_special; e != NULL; e = e->next) + { + if (e->addr == ptr) + { + break; + } + } + ASSERT(e); + ASSERT(e->addr == ptr); + e->addr = ret; + } + else if (!ptr) + { + /* sets e->addr to newptr */ + gc_addspecial(ret, free, a); + } + } + + return ret; +} + void x_gc_free(struct gc_arena *a) { diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index fece6336d..185226f7c 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -187,6 +187,19 @@ struct buffer string_alloc_buf(const char *str, struct gc_arena *gc); void gc_addspecial(void *addr, void (*free_function)(void *), struct gc_arena *a); +/** + * allows to realloc a pointer previously allocated by gc_malloc or gc_realloc + * + * @note only use this function on pointers returned by gc_malloc or re_alloc + * with the same gc_arena + * + * @param ptr Pointer of the previously allocated memory + * @param size New size + * @param a gc_arena to use + * @return new pointer + */ +void * +gc_realloc(void *ptr, size_t size, struct gc_arena *a); #ifdef BUF_INIT_TRACKING #define buf_init(buf, offset) buf_init_debug(buf, offset, __FILE__, __LINE__) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 4e018fb84..e454b2ac0 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -918,12 +918,10 @@ uninit_options(struct options *o) { if (o->connection_list) { - free(o->connection_list->array); CLEAR(*o->connection_list); } if (o->remote_list) { - free(o->remote_list->array); CLEAR(*o->remote_list); } if (o->gc_owned) @@ -2173,7 +2171,7 @@ alloc_connection_entry(struct options *options, const int msglevel) if (l->len == l->capacity) { int capacity = l->capacity + CONNECTION_LIST_SIZE; - struct connection_entry **ce = realloc(l->array, capacity*sizeof(struct connection_entry *)); + struct connection_entry **ce = gc_realloc(l->array, capacity*sizeof(struct connection_entry *), &options->gc); if (ce == NULL) { msg(msglevel, "Unable to process more connection options: out of memory. Number of entries = %d", l->len); @@ -2206,7 +2204,7 @@ alloc_remote_entry(struct options *options, const int msglevel) if (l->len == l->capacity) { int capacity = l->capacity + CONNECTION_LIST_SIZE; - struct remote_entry **re = realloc(l->array, capacity*sizeof(struct remote_entry *)); + struct remote_entry **re = gc_realloc(l->array, capacity*sizeof(struct remote_entry *), &options->gc); if (re == NULL) { msg(msglevel, "Unable to process more remote options: out of memory. Number of entries = %d", l->len); diff --git a/tests/unit_tests/openvpn/test_buffer.c b/tests/unit_tests/openvpn/test_buffer.c index ac701669f..9e3b1d2e9 100644 --- a/tests/unit_tests/openvpn/test_buffer.c +++ b/tests/unit_tests/openvpn/test_buffer.c @@ -231,6 +231,37 @@ test_buffer_free_gc_two(void **state) gc_free(&gc); } + +static void +test_buffer_gc_realloc(void **state) +{ + struct gc_arena gc = gc_new(); + + void *p1 = gc_realloc(NULL, 512, &gc); + void *p2 = gc_realloc(NULL, 512, &gc); + + assert_ptr_not_equal(p1, p2); + + memset(p1, '1', 512); + memset(p2, '2', 512); + + p1 = gc_realloc(p1, 512, &gc); + + /* allocate 512kB to ensure the pointer needs to change */ + void *p1new = gc_realloc(p1, 512ul * 1024, &gc); + assert_ptr_not_equal(p1, p1new); + + void *p2new = gc_realloc(p2, 512ul * 1024, &gc); + assert_ptr_not_equal(p2, p2new); + + void *p3 = gc_realloc(NULL, 512, &gc); + memset(p3, '3', 512); + + + gc_free(&gc); +} + + int main(void) { @@ -259,6 +290,7 @@ main(void) test_buffer_list_teardown), cmocka_unit_test(test_buffer_free_gc_one), cmocka_unit_test(test_buffer_free_gc_two), + cmocka_unit_test(test_buffer_gc_realloc), }; return cmocka_run_group_tests_name("buffer", tests, NULL, NULL);