From patchwork Fri Mar 6 13:05:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hongzhi Guo X-Patchwork-Id: 1250278 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.133; helo=hemlock.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 hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48YnvB1y76z9sSM for ; Sat, 7 Mar 2020 00:06:45 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 6633E85701; Fri, 6 Mar 2020 13:06:42 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NbQu7scq6g7V; Fri, 6 Mar 2020 13:06:39 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 544D6882D8; Fri, 6 Mar 2020 13:06:34 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3AEA5C18D3; Fri, 6 Mar 2020 13:06:34 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id B0415C013E for ; Fri, 6 Mar 2020 13:06:32 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 9910D8735E for ; Fri, 6 Mar 2020 13:06:32 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fAeYEUDwPbSY for ; Fri, 6 Mar 2020 13:06:30 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from huawei.com (szxga06-in.huawei.com [45.249.212.32]) by whitealder.osuosl.org (Postfix) with ESMTPS id 74B5586E59 for ; Fri, 6 Mar 2020 13:06:30 +0000 (UTC) Received: from DGGEMS413-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id C16C2C57DA899909A5E1; Fri, 6 Mar 2020 21:06:26 +0800 (CST) Received: from DESKTOP-ORJPOMD.china.huawei.com (10.173.251.143) by DGGEMS413-HUB.china.huawei.com (10.3.19.213) with Microsoft SMTP Server id 14.3.439.0; Fri, 6 Mar 2020 21:06:04 +0800 From: guohongzhi To: Date: Fri, 6 Mar 2020 21:05:55 +0800 Message-ID: <20200306130555.19884-1-guohongzhi1@huawei.com> X-Mailer: git-send-email 2.21.0.windows.1 MIME-Version: 1.0 X-Originating-IP: [10.173.251.143] X-CFilter-Loop: Reflected Cc: guohongzhi1@huawei.com, jerry.lilijun@huawei.com, zhoujingbin@huawei.com, chenchanghu@huawei.com Subject: [ovs-dev] [PATCH] ofproto:fix 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" ASAN report use-after-free when destroy ofproto_rule, the rule->ofproto has freed in ofproto_destroy. Add ref_count for ofproto to avoid use-after-free when destroy ofproto_rule adn group. Signed-off-by: guohongzhi --- ofproto/ofproto-dpif.c | 1 + ofproto/ofproto-provider.h | 3 +++ ofproto/ofproto.c | 31 +++++++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 7515352..258972d 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -1494,6 +1494,7 @@ construct(struct ofproto *ofproto_) ofproto->ams_seq = seq_create(); ofproto->ams_seqno = seq_read(ofproto->ams_seq); + ovs_refcount_init(&ofproto_->ref_count); SHASH_FOR_EACH_SAFE (node, next, &init_ofp_ports) { struct iface_hint *iface_hint = node->data; diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 7907d4b..c3e6527 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -139,6 +139,7 @@ struct ofproto { /* Variable length mf_field mapping. Stores all configured variable length * meta-flow fields (struct mf_field) in a switch. */ struct vl_mff_map vl_mff_map; + struct ovs_refcount ref_count; }; void ofproto_init_tables(struct ofproto *, int n_tables); @@ -442,6 +443,8 @@ struct rule { void ofproto_rule_ref(struct rule *); bool ofproto_rule_try_ref(struct rule *); void ofproto_rule_unref(struct rule *); +void ofproto_ref(struct ofproto *ofproto); +void ofproto_unref(struct ofproto *ofproto); static inline const struct rule_actions * rule_get_actions(const struct rule *); static inline bool rule_is_table_miss(const struct rule *); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 7b84784..dc9caa0 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -554,6 +554,10 @@ ofproto_create(const char *datapath_name, const char *datapath_type, ovs_mutex_unlock(&ofproto_mutex); ofproto_destroy__(ofproto); return error; + } else { + /* ofproto construct succeed, ref its self + * inorder to unref when call ofproto destroy*/ + ofproto_ref(ofproto); } /* Check that hidden tables, if any, are at the end. */ @@ -1673,8 +1677,9 @@ ofproto_destroy(struct ofproto *p, bool del) p->connmgr = NULL; ovs_mutex_unlock(&ofproto_mutex); - /* Destroying rules is deferred, must have 'ofproto' around for them. */ - ovsrcu_postpone(ofproto_destroy_defer__, p); + /* Inorder to keep the code logical as before, we ref for ofproto when ofproto create + * so, we should also unref ofproto here*/ + ofproto_unref(p); } /* Destroys the datapath with the respective 'name' and 'type'. With the Linux @@ -2852,6 +2857,22 @@ update_mtu_ofproto(struct ofproto *p) } } +void +ofproto_ref(struct ofproto *ofproto) +{ + if (ofproto) { + ovs_refcount_ref(&ofproto->ref_count); + } +} + +void +ofproto_unref(struct ofproto *ofproto) +{ + if (ofproto && ovs_refcount_unref_relaxed(&ofproto->ref_count) == 1) { + ovsrcu_postpone(ofproto_destroy_defer__, ofproto); + } +} + static void ofproto_rule_destroy__(struct rule *rule) OVS_NO_THREAD_SAFETY_ANALYSIS @@ -2860,6 +2881,7 @@ ofproto_rule_destroy__(struct rule *rule) rule_actions_destroy(rule_get_actions(rule)); ovs_mutex_destroy(&rule->mutex); rule->ofproto->ofproto_class->rule_dealloc(rule); + ofproto_unref(rule->ofproto); } static void @@ -3000,6 +3022,7 @@ group_destroy_cb(struct ofgroup *group) ofputil_bucket_list_destroy(CONST_CAST(struct ovs_list *, &group->buckets)); group->ofproto->ofproto_class->group_dealloc(group); + ofproto_unref(group->ofproto); } void @@ -5176,6 +5199,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct cls_rule *cr, /* Initialize base state. */ *CONST_CAST(struct ofproto **, &rule->ofproto) = ofproto; + ovs_refcount_ref(ofproto); cls_rule_move(CONST_CAST(struct cls_rule *, &rule->cr), cr); ovs_refcount_init(&rule->ref_count); @@ -7271,6 +7295,9 @@ init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm, ofputil_bucket_list_destroy(CONST_CAST(struct ovs_list *, &(*ofgroup)->buckets)); ofproto->ofproto_class->group_dealloc(*ofgroup); + } else { + /* group construct succeed, ref ofproto */ + ofproto_ref(ofproto); } return error; }