From patchwork Thu May 20 13:35:49 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gaetan Rivet X-Patchwork-Id: 1481621 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) 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=fm1 header.b=d1XJ9N/T; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.a=rsa-sha256 header.s=fm2 header.b=dsSSzXd4; dkim-atps=neutral Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::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 4Fm9k23C7Fz9sTD for ; Thu, 20 May 2021 23:36:10 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id D1E2283CCA; Thu, 20 May 2021 13:36:07 +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 QhfV4OQzRK_z; Thu, 20 May 2021 13:36:06 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTP id 037A083B0D; Thu, 20 May 2021 13:36:06 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C77DAC000D; Thu, 20 May 2021 13:36:05 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 279D3C0001 for ; Thu, 20 May 2021 13:36:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 165124049B for ; Thu, 20 May 2021 13:36:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp2.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=u256.net header.b="d1XJ9N/T"; dkim=pass (2048-bit key) header.d=messagingengine.com header.b="dsSSzXd4" Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id q0RCf0Czx9bu for ; Thu, 20 May 2021 13:36:01 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by smtp2.osuosl.org (Postfix) with ESMTPS id D25F3401B7 for ; Thu, 20 May 2021 13:36:01 +0000 (UTC) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 82B7C5C01E3 for ; Thu, 20 May 2021 09:36:00 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 20 May 2021 09:36:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=u256.net; h=from :to:subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=fm1; bh=lJ6+Jhax9UCDnizTO8oOunFeh8 8ZoBz10j40A7uY5iM=; b=d1XJ9N/TCwh9qJ/WFPb5QkUtXAy86FJUY3rf0r2Kyg XSUbEcfneyZFhuu8ID0NZY1fmJ8WnM1vkDUeqRH0uRrOEZ7t3XypXuo6nY6hkC7Y ciLI93andSKGmjfB8GMutbgaZqjTG095Dxft7ll8nHGSM/bmGE3LDUI8nORYGXz/ oRkUl11LmVfTEIraPV1LKLZpr0mJ2jpK0j404hEifmADmT0lhrOfxTA52OnmPQmJ 0hJcVbN5ynSAB5oaQocP3HxYG9DvanXuEemUHltgo7KW0G0TpUEZOcOGsAq0BrdI PiuIOj62utCFRwbati+6XDCyEM5TflB7QcGAt4k68Hwg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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= fm2; bh=lJ6+Jhax9UCDnizTO8oOunFeh88ZoBz10j40A7uY5iM=; b=dsSSzXd4 W5F8W0Ew0xtp8HvwN7d6gnmJzNIhuzYh/2B3yQnEw8JSeyKqAtutFfsjRfby7uvi gCid/KQCkrCfXGH8ZY1e+ZMMKW+FZhNrSTC/YKD1qFkSrdMmyJ0gUQYyv48DAQq+ iAuZEuY2hIPYF3KSJEQ7eVsNghXTJmkaijo0el23fDQpQfcH0mv24KTEhw0/XKgc q17jfXERjPCFVG/jFdNUWZPeibYzbRUj62tgBFaENW6glw0/RDEWWfuiXkVhsYj1 xr0dcai1aNb2brdi0kH73UbJcCnDTztTqf+enYoHH3UFiqBhp2QHkr3XMlaET9qW Yramrdi7Mza6GQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvdejuddgieejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpefhvffufffkofgjfhgggfestdekre dtredttdenucfhrhhomhepifgrvghtrghnucftihhvvghtuceoghhrihhvvgesuhdvheei rdhnvghtqeenucggtffrrghtthgvrhhnpeehgfevffekteehteefieefvdehleefjeefhe evudetjefhkeeutdekieeuvdetheenucfkphepkeeirddvheegrddvgeefrddugeeinecu vehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepghhrihhvvg esuhdvheeirdhnvght X-ME-Proxy: Received: from inocybe.home (lfbn-poi-1-930-146.w86-254.abo.wanadoo.fr [86.254.243.146]) by mail.messagingengine.com (Postfix) with ESMTPA for ; Thu, 20 May 2021 09:35:59 -0400 (EDT) From: Gaetan Rivet To: ovs-dev@openvswitch.org Date: Thu, 20 May 2021 15:35:49 +0200 Message-Id: X-Mailer: git-send-email 2.31.1 In-Reply-To: References: MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v2 5/8] 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 --- 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. */