From patchwork Wed Sep 8 09:47:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gaetan Rivet X-Patchwork-Id: 1525709 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=u256.net header.i=@u256.net header.a=rsa-sha256 header.s=fm2 header.b=WjOVNvy0; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.a=rsa-sha256 header.s=fm3 header.b=exjgGC+/; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 ozlabs.org (Postfix) with ESMTPS id 4H4HPn665Tz9t54 for ; Wed, 8 Sep 2021 19:48:13 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 3531F8315C; Wed, 8 Sep 2021 09:48:10 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WN9YaXK-pkR2; Wed, 8 Sep 2021 09:48:08 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id A4D2E82F84; Wed, 8 Sep 2021 09:48:06 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E7CBFC0020; Wed, 8 Sep 2021 09:48:04 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id BCDFCC000D for ; Wed, 8 Sep 2021 09:48:03 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id BA243606ED for ; Wed, 8 Sep 2021 09:48:03 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp3.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=u256.net header.b="WjOVNvy0"; dkim=pass (2048-bit key) header.d=messagingengine.com header.b="exjgGC+/" Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PLmQKkZPGxTG for ; Wed, 8 Sep 2021 09:48:03 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by smtp3.osuosl.org (Postfix) with ESMTPS id 20909606E7 for ; Wed, 8 Sep 2021 09:48:03 +0000 (UTC) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 878A732009B5; Wed, 8 Sep 2021 05:48:02 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Wed, 08 Sep 2021 05:48:02 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=u256.net; h=from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=fm2; bh=RfJR2N9ZtfI8g JnzZTpO+Rw+u0FSWJMgJE2t73bPgu8=; b=WjOVNvy0p/lskTF0vjDRPAjv4PuiR XTfV1yq/QlsQ+ZE3L94nyCFZ9AEEqLCCQxi8v5qHkEt32m/CHeEMI7pI0LSvZ0O2 LtEvaN5cE42Fk39X6c8FfF6G0k4Q2r4PGyMaxNw/+Q6FEHXEFRF4fzw4oWKCgB6M Be9VqpZIy5hUtX3A659AVpQ6EOTWijqQ2XTITNV+ehU14Vm2NGbNbexYrV+q0Lqz 7XidzTStZVmRqIW1VKVIyi3ExMkwMLqygXgm1pHRhhfAJyBO0vRVg4u5auLnM0Y0 RPfoml+iQOdJgkF2NeQnVbwAhvWd2wQ/LFbJq9AUnkcIC8e/EJ2/rkoeA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :in-reply-to:message-id:mime-version:references:subject:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; bh=RfJR2N9ZtfI8gJnzZTpO+Rw+u0FSWJMgJE2t73bPgu8=; b=exjgGC+/ cGNdoQ3U3ydFZfqDDwi2kT+NtObEGX0HdIGo69PBtY6rodEEfts6lZOJTsDWzyg8 TM+di6HLluUVr5yRl5nEgg/WjVf1Zo67nt33vgNzt8fbmFvWGhINyWlx3enMMROR OtBtiZZpaiEAjB4Cw6scqTBxMmcSCa7bb+Fx3Cf5rHs0P6W9sZzqH1byQtXr+QmW S5Lx544fwur18yDcYbPPjWtyyWAmsW2gYVpEmkIn1jhyQD4Z6DdVP4qEDPjpHFW8 LCdGi04POFda9ITNbmVhUAnXJyEZWuwngnpxO+XSlOJHbaOfsZ+NCO46ICk3kEqn eISnv0WCSvB0Tw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudefjedgudekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkffojghfggfgsedtkeertdertddtnecuhfhrohhmpefirggvthgr nhcutfhivhgvthcuoehgrhhivhgvsehuvdehiedrnhgvtheqnecuggftrfgrthhtvghrnh ephefgveffkeetheetfeeifedvheelfeejfeehveduteejhfekuedtkeeiuedvteehnecu vehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepghhrihhvvg esuhdvheeirdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 8 Sep 2021 05:48:01 -0400 (EDT) From: Gaetan Rivet To: ovs-dev@openvswitch.org Date: Wed, 8 Sep 2021 11:47:25 +0200 Message-Id: <37a5b181b7fd3d4b40ade357ed4c332acbaa1fe1.1631094144.git.grive@u256.net> X-Mailer: git-send-email 2.31.1 In-Reply-To: References: MIME-Version: 1.0 Cc: Maxime Coquelin Subject: [ovs-dev] [PATCH v5 01/27] ovs-thread: Fix barrier use-after-free X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" When a thread is blocked on a barrier, there is no guarantee regarding the moment it will resume, only that it will at some point in the future. One thread can resume first then proceed to destroy the barrier while another thread has not yet awoken. When it finally happens, the second thread will attempt a seq_read() on the barrier seq, while the first thread have already destroyed it, triggering a use-after-free. Introduce an additional indirection layer within the barrier. A internal barrier implementation holds all the necessary elements for a thread to safely block and destroy. Whenever a barrier is destroyed, the internal implementation is left available to still blocking threads if necessary. A reference counter is used to track threads still using the implementation. Note that current uses of ovs-barrier are not affected: RCU and revalidators will not destroy their barrier immediately after blocking on it. Fixes: d8043da7182a ("ovs-thread: Implement OVS specific barrier.") Signed-off-by: Gaetan Rivet Reviewed-by: Maxime Coquelin --- lib/ovs-thread.c | 61 +++++++++++++++++++++++++++++++++++++++--------- lib/ovs-thread.h | 6 ++--- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c index b686e4548..805cba622 100644 --- a/lib/ovs-thread.c +++ b/lib/ovs-thread.c @@ -299,21 +299,53 @@ ovs_spin_init(const struct ovs_spin *spin) } #endif +struct ovs_barrier_impl { + uint32_t size; /* Number of threads to wait. */ + atomic_count count; /* Number of threads already hit the barrier. */ + struct seq *seq; + struct ovs_refcount refcnt; +}; + +static void +ovs_barrier_impl_ref(struct ovs_barrier_impl *impl) +{ + ovs_refcount_ref(&impl->refcnt); +} + +static void +ovs_barrier_impl_unref(struct ovs_barrier_impl *impl) +{ + if (ovs_refcount_unref(&impl->refcnt) == 1) { + seq_destroy(impl->seq); + free(impl); + } +} + /* Initializes the 'barrier'. 'size' is the number of threads * expected to hit the barrier. */ void ovs_barrier_init(struct ovs_barrier *barrier, uint32_t size) { - barrier->size = size; - atomic_count_init(&barrier->count, 0); - barrier->seq = seq_create(); + struct ovs_barrier_impl *impl; + + impl = xmalloc(sizeof *impl); + impl->size = size; + atomic_count_init(&impl->count, 0); + impl->seq = seq_create(); + ovs_refcount_init(&impl->refcnt); + + ovsrcu_set(&barrier->impl, impl); } /* Destroys the 'barrier'. */ void ovs_barrier_destroy(struct ovs_barrier *barrier) { - seq_destroy(barrier->seq); + struct ovs_barrier_impl *impl; + + impl = ovsrcu_get(struct ovs_barrier_impl *, &barrier->impl); + ovsrcu_set(&barrier->impl, NULL); + ovs_barrier_impl_unref(impl); } /* Makes the calling thread block on the 'barrier' until all @@ -325,23 +357,30 @@ ovs_barrier_destroy(struct ovs_barrier *barrier) void ovs_barrier_block(struct ovs_barrier *barrier) { - uint64_t seq = seq_read(barrier->seq); + struct ovs_barrier_impl *impl; uint32_t orig; + uint64_t seq; - orig = atomic_count_inc(&barrier->count); - if (orig + 1 == barrier->size) { - atomic_count_set(&barrier->count, 0); + impl = ovsrcu_get(struct ovs_barrier_impl *, &barrier->impl); + ovs_barrier_impl_ref(impl); + + seq = seq_read(impl->seq); + orig = atomic_count_inc(&impl->count); + if (orig + 1 == impl->size) { + atomic_count_set(&impl->count, 0); /* seq_change() serves as a release barrier against the other threads, * so the zeroed count is visible to them as they continue. */ - seq_change(barrier->seq); + seq_change(impl->seq); } else { /* To prevent thread from waking up by other event, * keeps waiting for the change of 'barrier->seq'. */ - while (seq == seq_read(barrier->seq)) { - seq_wait(barrier->seq, seq); + while (seq == seq_read(impl->seq)) { + seq_wait(impl->seq, seq); poll_block(); } } + + ovs_barrier_impl_unref(impl); } DEFINE_EXTERN_PER_THREAD_DATA(ovsthread_id, OVSTHREAD_ID_UNSET); diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h index 7ee98bd4e..3b444ccdc 100644 --- a/lib/ovs-thread.h +++ b/lib/ovs-thread.h @@ -21,16 +21,16 @@ #include #include #include "ovs-atomic.h" +#include "ovs-rcu.h" #include "openvswitch/thread.h" #include "util.h" struct seq; /* Poll-block()-able barrier similar to pthread_barrier_t. */ +struct ovs_barrier_impl; struct ovs_barrier { - uint32_t size; /* Number of threads to wait. */ - atomic_count count; /* Number of threads already hit the barrier. */ - struct seq *seq; + OVSRCU_TYPE(struct ovs_barrier_impl *) impl; }; /* Wrappers for pthread_mutexattr_*() that abort the process on any error. */