netdev - [PATCH net-next v3 1/3] net: dsa: Ensure that port array elements are initialized before being used

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 24 Feb 2015 13:15:32 -0800
From:	Florian Fainelli <f.fainelli@...il.com>
To:	netdev@...r.kernel.org
Cc:	davem@...emloft.net, Guenter Roeck <linux@...ck-us.net>,
	vivien.didelot@...oirfairelinux.com,
	jerome.oufella@...oirfairelinux.com, andrew@...n.ch,
	cphealy@...il.com
Subject: [PATCH net-next v3 1/3] net: dsa: Ensure that port array elements are initialized before being used

From: Guenter Roeck <linux@...ck-us.net>

A network device notifier can be called for one or more of the created
slave devices before all slave devices have been registered. This can
result in a mismatch between ds->phys_port_mask and the registered devices
by the time the call is made, and it can result in a slave device being
added to a bridge before its entry in ds->ports[] has been initialized.

Rework the initialization code to initialize entries in ds->ports[] in
dsa_slave_create. With this change, dsa_slave_create no longer needs
to return slave_dev but can return an error code instead.

Signed-off-by: Guenter Roeck <linux@...ck-us.net>
---
Changes in v2:
- slightly rework the commit message so it does not mention the
  netdevice_notifier that we are adding in the next patch

 net/dsa/dsa.c      | 10 +++-------
 net/dsa/dsa_priv.h |  5 ++---
 net/dsa/slave.c    | 15 ++++++++-------
 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 2173402d87e0..fc1813140be6 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -314,19 +314,15 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	 * Create network devices for physical switch ports.
 	 */
 	for (i = 0; i < DSA_MAX_PORTS; i++) {
-		struct net_device *slave_dev;
-
 		if (!(ds->phys_port_mask & (1 << i)))
 			continue;
 
-		slave_dev = dsa_slave_create(ds, parent, i, pd->port_names[i]);
-		if (slave_dev == NULL) {
+		ret = dsa_slave_create(ds, parent, i, pd->port_names[i]);
+		if (ret < 0) {
 			netdev_err(dst->master_netdev, "[%d]: can't create dsa slave device for port %d(%s)\n",
 				   index, i, pd->port_names[i]);
-			continue;
+			ret = 0;
 		}
-
-		ds->ports[i] = slave_dev;
 	}
 
 #ifdef CONFIG_NET_DSA_HWMON
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index dc9756d3154c..7eb1a6acd46c 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -53,9 +53,8 @@ extern char dsa_driver_version[];
 /* slave.c */
 extern const struct dsa_device_ops notag_netdev_ops;
 void dsa_slave_mii_bus_init(struct dsa_switch *ds);
-struct net_device *dsa_slave_create(struct dsa_switch *ds,
-				    struct device *parent,
-				    int port, char *name);
+int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
+		     int port, char *name);
 int dsa_slave_suspend(struct net_device *slave_dev);
 int dsa_slave_resume(struct net_device *slave_dev);
 
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index f23deadf42a0..5be4c928c9c9 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -605,9 +605,8 @@ int dsa_slave_resume(struct net_device *slave_dev)
 	return 0;
 }
 
-struct net_device *
-dsa_slave_create(struct dsa_switch *ds, struct device *parent,
-		 int port, char *name)
+int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
+		     int port, char *name)
 {
 	struct net_device *master = ds->dst->master_netdev;
 	struct net_device *slave_dev;
@@ -617,7 +616,7 @@ dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 	slave_dev = alloc_netdev(sizeof(struct dsa_slave_priv), name,
 				 NET_NAME_UNKNOWN, ether_setup);
 	if (slave_dev == NULL)
-		return slave_dev;
+		return -ENOMEM;
 
 	slave_dev->features = master->vlan_features;
 	slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
@@ -667,19 +666,21 @@ dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 	ret = dsa_slave_phy_setup(p, slave_dev);
 	if (ret) {
 		free_netdev(slave_dev);
-		return NULL;
+		return ret;
 	}
 
+	ds->ports[port] = slave_dev;
 	ret = register_netdev(slave_dev);
 	if (ret) {
 		netdev_err(master, "error %d registering interface %s\n",
 			   ret, slave_dev->name);
 		phy_disconnect(p->phy);
+		ds->ports[port] = NULL;
 		free_netdev(slave_dev);
-		return NULL;
+		return ret;
 	}
 
 	netif_carrier_off(slave_dev);
 
-	return slave_dev;
+	return 0;
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html