From patchwork Mon Jun 1 02:26:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linhaifeng X-Patchwork-Id: 1301564 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=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=huawei.com Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49ZzZQ4dxwz9sSf for ; Mon, 1 Jun 2020 12:26:37 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id BBD2385F10; Mon, 1 Jun 2020 02:26:35 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2ZdW1XkLYOQW; Mon, 1 Jun 2020 02:26:33 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id D6C1C85C4C; Mon, 1 Jun 2020 02:26:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B9147C088C; Mon, 1 Jun 2020 02:26:33 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 18691C0176 for ; Mon, 1 Jun 2020 02:26:32 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id E5D8720343 for ; Mon, 1 Jun 2020 02:26:31 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jY6zZgpXAKfJ for ; Mon, 1 Jun 2020 02:26:30 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from huawei.com (szxga02-in.huawei.com [45.249.212.188]) by silver.osuosl.org (Postfix) with ESMTPS id C1EA820133 for ; Mon, 1 Jun 2020 02:26:29 +0000 (UTC) Received: from dggeml405-hub.china.huawei.com (unknown [172.30.72.54]) by Forcepoint Email with ESMTP id 73DE41F2C2B9D1DFEFDB; Mon, 1 Jun 2020 10:26:26 +0800 (CST) Received: from DGGEML522-MBS.china.huawei.com ([169.254.8.148]) by dggeml405-hub.china.huawei.com ([10.3.17.49]) with mapi id 14.03.0487.000; Mon, 1 Jun 2020 10:26:19 +0800 From: Linhaifeng To: Ben Pfaff Thread-Topic: [PATCH] ovs rcu: update rcu pointer first Thread-Index: AdY3u2R8V4WofMbTQ026GAqpjkRqcg== Date: Mon, 1 Jun 2020 02:26:17 +0000 Message-ID: <4099DE2E54AFAD489356C6C9161D533397358B01@dggeml522-mbs.china.huawei.com> Accept-Language: en-GB, zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.173.251.242] MIME-Version: 1.0 X-CFilter-Loop: Reflected Cc: "dev@openvswitch.org" , "Lilijun \(Jerry\)" , chenchanghu Subject: [ovs-dev] [PATCH] ovs rcu: update rcu pointer first 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" We should update rcu pointer first then use ovsrcu_postpone to free otherwise maybe cause use-after-free. e.g, thead are two threads, A and B: 1. thread A call ovsrcu_postpone and flush cbset, have not call ovsrcu_quiesce 2. thread rcu wait all threads call ovsrcu_quiesce 3. thread B call ovsrcu_quiesce 4. thread B next round get the old pointer 5. thrad A call ovsrcu_quiesce 6. thread rcu free old pointer 7. thread B use-after-free Signed-off-by: Haifeng Lin --- lib/classifier.c | 4 ++-- lib/ovs-rcu.h | 2 +- lib/pvector.c | 15 ++++++++------- ofproto/ofproto-dpif-mirror.c | 4 ++-- ofproto/ofproto-dpif-upcall.c | 3 +-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/classifier.c b/lib/classifier.c index f2c3497c2..6bff76e07 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -249,11 +249,11 @@ cls_rule_set_conjunctions(struct cls_rule *cr, unsigned int old_n = old ? old->n : 0; if (old_n != n || (n && memcmp(old_conj, conj, n * sizeof *conj))) { + ovsrcu_set(&match->conj_set, + cls_conjunction_set_alloc(match, conj, n)); if (old) { ovsrcu_postpone(free, old); } - ovsrcu_set(&match->conj_set, - cls_conjunction_set_alloc(match, conj, n)); } } diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h index ecc4c9201..a66d868ea 100644 --- a/lib/ovs-rcu.h +++ b/lib/ovs-rcu.h @@ -119,9 +119,9 @@ * change_flow(struct flow *new_flow) * { * ovs_mutex_lock(&mutex); + * ovsrcu_set(&flowp, new_flow); * ovsrcu_postpone(free, * ovsrcu_get_protected(struct flow *, &flowp)); - * ovsrcu_set(&flowp, new_flow); * ovs_mutex_unlock(&mutex); * } * diff --git a/lib/pvector.c b/lib/pvector.c index cc527fdc4..6a295ec53 100644 --- a/lib/pvector.c +++ b/lib/pvector.c @@ -67,10 +67,11 @@ pvector_init(struct pvector *pvec) void pvector_destroy(struct pvector *pvec) { + struct pvector_impl *old = pvector_impl_get(pvec); free(pvec->temp); pvec->temp = NULL; - ovsrcu_postpone(free, pvector_impl_get(pvec)); ovsrcu_set(&pvec->impl, NULL); /* Poison. */ + ovsrcu_postpone(free, old); } /* Iterators for callers that need the 'index' afterward. */ @@ -205,11 +206,11 @@ pvector_change_priority(struct pvector *pvec, void *ptr, int priority) /* Make the modified pvector available for iteration. */ void pvector_publish__(struct pvector *pvec) { - struct pvector_impl *temp = pvec->temp; - + struct pvector_impl *new = pvec->temp; + struct pvector_impl *old = vsrcu_get_protected(struct pvector_impl *, + &pvec->impl); pvec->temp = NULL; - pvector_impl_sort(temp); /* Also removes gaps. */ - ovsrcu_postpone(free, ovsrcu_get_protected(struct pvector_impl *, - &pvec->impl)); - ovsrcu_set(&pvec->impl, temp); + pvector_impl_sort(new); /* Also removes gaps. */ + ovsrcu_set(&pvec->impl, new); + ovsrcu_postpone(free, old); } diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c index 343b75f0e..343100c08 100644 --- a/ofproto/ofproto-dpif-mirror.c +++ b/ofproto/ofproto-dpif-mirror.c @@ -276,9 +276,9 @@ mirror_set(struct mbridge *mbridge, void *aux, const char *name, hmapx_destroy(&dsts_map); if (vlans || src_vlans) { + unsigned long *new_vlans = vlan_bitmap_clone(src_vlans); + ovsrcu_set(&mirror->vlans, new_vlans); ovsrcu_postpone(free, vlans); - vlans = vlan_bitmap_clone(src_vlans); - ovsrcu_set(&mirror->vlans, vlans); } mirror->out = out; diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 5e08ef10d..be6dafb78 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1658,11 +1658,10 @@ ukey_set_actions(struct udpif_key *ukey, const struct ofpbuf *actions) struct ofpbuf *old_actions = ovsrcu_get_protected(struct ofpbuf *, &ukey->actions); + ovsrcu_set(&ukey->actions, ofpbuf_clone(actions)); if (old_actions) { ovsrcu_postpone(ofpbuf_delete, old_actions); } - - ovsrcu_set(&ukey->actions, ofpbuf_clone(actions)); } static struct udpif_key *