linux-kernel - [PATCH net-next] net: dsa: remove tree refcount

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20171109173652.25258-1-vivien.didelot@savoirfairelinux.com>
Date:   Thu,  9 Nov 2017 12:36:52 -0500
From:   Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To:     netdev@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, kernel@...oirfairelinux.com,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        Stephen Rothwell <sfr@...b.auug.org.au>, lkp@...org,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Subject: [PATCH net-next] net: dsa: remove tree refcount

Setting the refcount to 0 when allocating a tree to match the number of
switch devices it holds may cause an 'increment on 0; use-after-free'.

Tracking the number of devices in a tree with a kref is not really
appropriate anyway so removes it completely in favor of a basic counter.

Fixes: 8e5bf9759a06 ("net: dsa: simplify tree reference counting")
Signed-off-by: Vivien Didelot <vivien.didelot@...oirfairelinux.com>
---
 include/net/dsa.h |  2 +-
 net/dsa/dsa2.c    | 29 ++++-------------------------
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 35b5dee7bb23..bbcdff43526f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -119,7 +119,7 @@ struct dsa_switch_tree {
 	unsigned int index;
 
 	/* Number of switches attached to this tree */
-	struct kref refcount;
+	unsigned int nr_devices;
 
 	/* Has this tree been applied to the hardware? */
 	bool setup;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index fd54a8e17986..fda3e5415eaf 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -51,10 +51,6 @@ static struct dsa_switch_tree *dsa_tree_alloc(int index)
 	INIT_LIST_HEAD(&dst->list);
 	list_add_tail(&dsa_tree_list, &dst->list);
 
-	/* Initialize the reference counter to the number of switches, not 1 */
-	kref_init(&dst->refcount);
-	refcount_set(&dst->refcount.refcount, 0);
-
 	return dst;
 }
 
@@ -75,25 +71,6 @@ static struct dsa_switch_tree *dsa_tree_touch(int index)
 	return dst;
 }
 
-static void dsa_tree_get(struct dsa_switch_tree *dst)
-{
-	kref_get(&dst->refcount);
-}
-
-static void dsa_tree_release(struct kref *ref)
-{
-	struct dsa_switch_tree *dst;
-
-	dst = container_of(ref, struct dsa_switch_tree, refcount);
-
-	dsa_tree_free(dst);
-}
-
-static void dsa_tree_put(struct dsa_switch_tree *dst)
-{
-	kref_put(&dst->refcount, dsa_tree_release);
-}
-
 static bool dsa_port_is_dsa(struct dsa_port *port)
 {
 	return port->type == DSA_PORT_TYPE_DSA;
@@ -492,7 +469,9 @@ static void dsa_tree_remove_switch(struct dsa_switch_tree *dst,
 	dsa_tree_teardown(dst);
 
 	dst->ds[index] = NULL;
-	dsa_tree_put(dst);
+	dst->nr_devices--;
+	if (!dst->nr_devices)
+		dsa_tree_free(dst);
 }
 
 static int dsa_tree_add_switch(struct dsa_switch_tree *dst,
@@ -504,7 +483,7 @@ static int dsa_tree_add_switch(struct dsa_switch_tree *dst,
 	if (dst->ds[index])
 		return -EBUSY;
 
-	dsa_tree_get(dst);
+	dst->nr_devices++;
 	dst->ds[index] = ds;
 
 	err = dsa_tree_setup(dst);
-- 
2.15.0