From patchwork Wed Sep 16 17:32:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gregory Rose X-Patchwork-Id: 1365752 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=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=cGtzYOF1; dkim-atps=neutral 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 4BsJy330Wmz9sVn for ; Thu, 17 Sep 2020 11:18:35 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id DD14C86C35; Wed, 16 Sep 2020 17:33:57 +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 5xh0xQ0BLrBO; Wed, 16 Sep 2020 17:33:54 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id EAC4286CB8; Wed, 16 Sep 2020 17:33:34 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B4C58C0864; Wed, 16 Sep 2020 17:33:34 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 64E74C0893 for ; Wed, 16 Sep 2020 17:33:33 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 1D25687482 for ; Wed, 16 Sep 2020 17:33:33 +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 yZWAN4tk-lUR for ; Wed, 16 Sep 2020 17:33:29 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-pl1-f194.google.com (mail-pl1-f194.google.com [209.85.214.194]) by hemlock.osuosl.org (Postfix) with ESMTPS id C7B0487448 for ; Wed, 16 Sep 2020 17:33:28 +0000 (UTC) Received: by mail-pl1-f194.google.com with SMTP id k13so3567137plk.3 for ; Wed, 16 Sep 2020 10:33:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=0TlSA/haXeR/N72OY3Kr2G8N1P392hRuJ8ao8hyZF5A=; b=cGtzYOF1ELTObyuE0ol/pEOKvDQDjaFCxlKQA8TaykczP2A0TYl1VsFKjoJj0seJcx h675OXtFIF5ncVxqFmDqawgjGTfPAegnKqn09FrEnf87+gcUOEXMrlij1/kS8ljj2YOW Ft/37CIq5pLYucSBQRZGDPnacDQHVEdobwQ/UFP9GtpKQNEvuR7FW+16Ps0S49ye2wM9 26kYgDbgwTwS/0fd8+fxSQTLt6cxfxhJ7j8qEnOKyj1TIx86lPEhdWDUZ2gXtUquACwy W3smvzkqCBtW70kzmfm+qb7dgIu7UEemkjjpfB5eJThWogb046V5XUtQu216UvOrPCyr MUUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=0TlSA/haXeR/N72OY3Kr2G8N1P392hRuJ8ao8hyZF5A=; b=XrKtUbZxpz2IYLruqFh1vmEFHHOMMB38x/e6kvkEjnT9xd+VR3eVQNXHF5DBVaYXCC SHDQp7owACFMaqfudzuB2JnnUmce9INBNBhSKX/arvwDn0Axd6W+MhqjtG5E9E0YqPez keItYDXhudLWGsWZKUynRJqBI5Ey5fuX1ki5l5pZIVVZC4WJaDI1zX9756w+uqIzlVmW SvYvyE8awuZMqqL+66eWn/5LYyR6sO8MwX3fSsnJF8cWSj1RwGdSnvmiFrRh29P88UiH nN1V+Hq19IdqSAJtVx5e8AN+AAdiURqLMMsiGjth+gJXA9OMLOBq3BHnvfBhybydnhTS r/nQ== X-Gm-Message-State: AOAM532lPcfB8O0FIEfRKSw/Z1E7SksARTiIjdhb35zrBQdx1GhlA/GO sy4aI/OxNyUrzeSfI/WQZA7BheU+gOf9DQ== X-Google-Smtp-Source: ABdhPJxl6RgzgPQAgXybry82OZypBAD6ogIk7ZeiBg1cmiQRKvaNg0cR4KEEjZyO9uOWjquCntnfsg== X-Received: by 2002:a17:902:a405:b029:d1:e598:3ffc with SMTP id p5-20020a170902a405b02900d1e5983ffcmr7537589plq.54.1600277607595; Wed, 16 Sep 2020 10:33:27 -0700 (PDT) Received: from VMware-box.domain ([97.115.183.169]) by smtp.googlemail.com with ESMTPSA id o20sm16248519pgh.63.2020.09.16.10.33.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Sep 2020 10:33:26 -0700 (PDT) From: Greg Rose To: dev@openvswitch.org Date: Wed, 16 Sep 2020 10:32:58 -0700 Message-Id: <20200916173311.30956-12-gvrose8192@gmail.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200916173311.30956-1-gvrose8192@gmail.com> References: <20200916173311.30956-1-gvrose8192@gmail.com> Subject: [ovs-dev] [PATCH V3 11/24] datapath: fix possible memleak on destroy flow-table 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Tonghao Zhang Upstream commit: commit 50b0e61b32ee890a75b4377d5fbe770a86d6a4c1 Author: Tonghao Zhang Date: Fri Nov 1 22:23:52 2019 +0800 net: openvswitch: fix possible memleak on destroy flow-table When we destroy the flow tables which may contain the flow_mask, so release the flow mask struct. Signed-off-by: Tonghao Zhang Tested-by: Greg Rose Acked-by: Pravin B Shelar Signed-off-by: David S. Miller Added additional compat layer fixup for WRITE_ONCE() Cc: Tonghao Zhang Reviewed-by: Tonghao Zhang Signed-off-by: Greg Rose --- datapath/flow_table.c | 186 +++++++++--------- .../linux/compat/include/linux/compiler.h | 13 ++ 2 files changed, 111 insertions(+), 88 deletions(-) diff --git a/datapath/flow_table.c b/datapath/flow_table.c index ca2efe94d..bd05dd394 100644 --- a/datapath/flow_table.c +++ b/datapath/flow_table.c @@ -234,6 +234,74 @@ static int tbl_mask_array_realloc(struct flow_table *tbl, int size) return 0; } +static int tbl_mask_array_add_mask(struct flow_table *tbl, + struct sw_flow_mask *new) +{ + struct mask_array *ma = ovsl_dereference(tbl->mask_array); + int err, ma_count = READ_ONCE(ma->count); + + if (ma_count >= ma->max) { + err = tbl_mask_array_realloc(tbl, ma->max + + MASK_ARRAY_SIZE_MIN); + if (err) + return err; + + ma = ovsl_dereference(tbl->mask_array); + } + + BUG_ON(ovsl_dereference(ma->masks[ma_count])); + + rcu_assign_pointer(ma->masks[ma_count], new); + WRITE_ONCE(ma->count, ma_count +1); + + return 0; +} + +static void tbl_mask_array_del_mask(struct flow_table *tbl, + struct sw_flow_mask *mask) +{ + struct mask_array *ma = ovsl_dereference(tbl->mask_array); + int i, ma_count = READ_ONCE(ma->count); + + /* Remove the deleted mask pointers from the array */ + for (i = 0; i < ma_count; i++) { + if (mask == ovsl_dereference(ma->masks[i])) + goto found; + } + + BUG(); + return; + +found: + WRITE_ONCE(ma->count, ma_count -1); + + rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]); + RCU_INIT_POINTER(ma->masks[ma_count -1], NULL); + + kfree_rcu(mask, rcu); + + /* Shrink the mask array if necessary. */ + if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) && + ma_count <= (ma->max / 3)) + tbl_mask_array_realloc(tbl, ma->max / 2); +} + +/* Remove 'mask' from the mask list, if it is not needed any more. */ +static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask) +{ + if (mask) { + /* ovs-lock is required to protect mask-refcount and + * mask list. + */ + ASSERT_OVSL(); + BUG_ON(!mask->ref_count); + mask->ref_count--; + + if (!mask->ref_count) + tbl_mask_array_del_mask(tbl, mask); + } +} + int ovs_flow_tbl_init(struct flow_table *table) { struct table_instance *ti, *ufid_ti; @@ -280,7 +348,28 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu) __table_instance_destroy(ti); } -static void table_instance_destroy(struct table_instance *ti, +static void table_instance_flow_free(struct flow_table *table, + struct table_instance *ti, + struct table_instance *ufid_ti, + struct sw_flow *flow, + bool count) +{ + hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); + if (count) + table->count--; + + if (ovs_identifier_is_ufid(&flow->id)) { + hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); + + if (count) + table->ufid_count--; + } + + flow_mask_remove(table, flow->mask); +} + +static void table_instance_destroy(struct flow_table *table, + struct table_instance *ti, struct table_instance *ufid_ti, bool deferred) { @@ -297,13 +386,12 @@ static void table_instance_destroy(struct table_instance *ti, struct sw_flow *flow; struct hlist_head *head = &ti->buckets[i]; struct hlist_node *n; - int ver = ti->node_ver; - int ufid_ver = ufid_ti->node_ver; - hlist_for_each_entry_safe(flow, n, head, flow_table.node[ver]) { - hlist_del_rcu(&flow->flow_table.node[ver]); - if (ovs_identifier_is_ufid(&flow->id)) - hlist_del_rcu(&flow->ufid_table.node[ufid_ver]); + hlist_for_each_entry_safe(flow, n, head, + flow_table.node[ti->node_ver]) { + + table_instance_flow_free(table, ti, ufid_ti, + flow, false); ovs_flow_free(flow, deferred); } } @@ -328,7 +416,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table) free_percpu(table->mask_cache); kfree(rcu_dereference_raw(table->mask_array)); - table_instance_destroy(ti, ufid_ti, false); + table_instance_destroy(table, ti, ufid_ti, false); } struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti, @@ -444,7 +532,7 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table) flow_table->count = 0; flow_table->ufid_count = 0; - table_instance_destroy(old_ti, old_ufid_ti, true); + table_instance_destroy(flow_table, old_ti, old_ufid_ti, true); return 0; err_free_ti: @@ -722,51 +810,6 @@ static struct table_instance *table_instance_expand(struct table_instance *ti, return table_instance_rehash(ti, ti->n_buckets * 2, ufid); } -static void tbl_mask_array_del_mask(struct flow_table *tbl, - struct sw_flow_mask *mask) -{ - struct mask_array *ma = ovsl_dereference(tbl->mask_array); - int i, ma_count = READ_ONCE(ma->count); - - /* Remove the deleted mask pointers from the array */ - for (i = 0; i < ma_count; i++) { - if (mask == ovsl_dereference(ma->masks[i])) - goto found; - } - - BUG(); - return; - -found: - WRITE_ONCE(ma->count, ma_count -1); - - rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]); - RCU_INIT_POINTER(ma->masks[ma_count -1], NULL); - - kfree_rcu(mask, rcu); - - /* Shrink the mask array if necessary. */ - if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) && - ma_count <= (ma->max / 3)) - tbl_mask_array_realloc(tbl, ma->max / 2); -} - -/* Remove 'mask' from the mask list, if it is not needed any more. */ -static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask) -{ - if (mask) { - /* ovs-lock is required to protect mask-refcount and - * mask list. - */ - ASSERT_OVSL(); - BUG_ON(!mask->ref_count); - mask->ref_count--; - - if (!mask->ref_count) - tbl_mask_array_del_mask(tbl, mask); - } -} - /* Must be called with OVS mutex held. */ void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) { @@ -774,17 +817,7 @@ void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) struct table_instance *ufid_ti = ovsl_dereference(table->ufid_ti); BUG_ON(table->count == 0); - hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); - table->count--; - if (ovs_identifier_is_ufid(&flow->id)) { - hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); - table->ufid_count--; - } - - /* RCU delete the mask. 'flow->mask' is not NULLed, as it should be - * accessible as long as the RCU read lock is held. - */ - flow_mask_remove(table, flow->mask); + table_instance_flow_free(table, ti, ufid_ti, flow, true); } static struct sw_flow_mask *mask_alloc(void) @@ -827,29 +860,6 @@ static struct sw_flow_mask *flow_mask_find(const struct flow_table *tbl, return NULL; } -static int tbl_mask_array_add_mask(struct flow_table *tbl, - struct sw_flow_mask *new) -{ - struct mask_array *ma = ovsl_dereference(tbl->mask_array); - int err, ma_count = READ_ONCE(ma->count); - - if (ma_count >= ma->max) { - err = tbl_mask_array_realloc(tbl, ma->max + - MASK_ARRAY_SIZE_MIN); - if (err) - return err; - - ma = ovsl_dereference(tbl->mask_array); - } - - BUG_ON(ovsl_dereference(ma->masks[ma_count])); - - rcu_assign_pointer(ma->masks[ma_count], new); - WRITE_ONCE(ma->count, ma_count +1); - - return 0; -} - /* Add 'mask' into the mask list, if it is not already there. */ static int flow_mask_insert(struct flow_table *tbl, struct sw_flow *flow, const struct sw_flow_mask *new) diff --git a/datapath/linux/compat/include/linux/compiler.h b/datapath/linux/compat/include/linux/compiler.h index 65f3ba6f4..e15c61d2a 100644 --- a/datapath/linux/compat/include/linux/compiler.h +++ b/datapath/linux/compat/include/linux/compiler.h @@ -15,4 +15,17 @@ #define READ_ONCE(x) (x) #endif +#ifndef WRITE_ONCE +#define __WRITE_ONCE(x, val) \ +do { \ + *(volatile typeof(x) *)&(x) = (val); \ +} while (0) + +#define WRITE_ONCE(x, val) \ +do { \ + __WRITE_ONCE(x, val); \ +} while (0) +#endif + + #endif